Skip to content

Conversation

@NamesMT
Copy link
Contributor

@NamesMT NamesMT commented Oct 24, 2025

🔗 Linked issue

📚 Description

The user might want to turn off the interval clock to save resources, especially on a page that renders a lot of relative time elements.

The PR adds a new relativeReactive prop to enable/disable the interval clock for NuxtTime relative render, supporting dynamic toggling of the clock, adding and updating tests, and note it to the docs.

@NamesMT NamesMT requested a review from danielroe as a code owner October 24, 2025 05:14
@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

Walkthrough

A new boolean prop relativeReactive (exposed as relative-reactive) was added to the NuxtTime component to control whether relative time labels update automatically; it defaults to true. The component's interval-based update logic was replaced with a reactive-controlled interval lifecycle that starts or stops based on this prop and is cleaned up on unmount. Documentation examples were updated to show disabling reactive updates, and tests were added/updated to verify reactive and non-reactive behaviour and SSR hydration attributes (data-relative-style and data-relative-reactive).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title feat(nuxt): NuxtTime: add relativeReactive prop is clear, specific, and directly aligned with the main change in the changeset. The title accurately describes the primary feature being added—a new relativeReactive prop to the NuxtTime component—which is reflected across all three modified files: the component implementation, documentation, and tests. The conventional commit format with the "feat" prefix also signals this is an enhancement, and the title avoids vague terminology.
Description Check ✅ Passed The pull request description is well-related to the changeset. It clearly explains the motivation (saving resources on pages with many relative time elements), describes the solution (adding the relativeReactive prop to enable/disable the interval clock), mentions support for dynamic toggling, and notes that tests and documentation are being updated. All of these points correspond directly to the changes present in the changeset, making the description sufficiently detailed and on-topic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d930a8e and a9545b4.

📒 Files selected for processing (1)
  • test/nuxt/nuxt-time.test.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/nuxt/nuxt-time.test.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/3.api/1.components/13.nuxt-time.md (1)

99-102: Clear documentation with an optional structural improvement.

The documentation clearly explains the relative-reactive prop and its usage. The example effectively demonstrates disabling automatic updates.

For improved discoverability, consider adding a dedicated subsection for relative-reactive under "Relative Time Formatting Props" (similar to how datetime and locale have dedicated sections), rather than only documenting it inline within the relative example. This would follow the established pattern and make the prop more discoverable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56a257a and d930a8e.

📒 Files selected for processing (3)
  • docs/3.api/1.components/13.nuxt-time.md (1 hunks)
  • packages/nuxt/src/app/components/nuxt-time.vue (3 hunks)
  • test/nuxt/nuxt-time.test.ts (5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Follow standard TypeScript conventions and best practices

Files:

  • test/nuxt/nuxt-time.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Write unit tests for core functionality using vitest

Files:

  • test/nuxt/nuxt-time.test.ts
**/*.vue

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use <script setup lang="ts"> and the composition API when creating Vue components

Files:

  • packages/nuxt/src/app/components/nuxt-time.vue
🧠 Learnings (1)
📚 Learning: 2025-07-18T16:46:07.446Z
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T16:46:07.446Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components

Applied to files:

  • packages/nuxt/src/app/components/nuxt-time.vue
🔇 Additional comments (5)
packages/nuxt/src/app/components/nuxt-time.vue (3)

2-2: LGTM!

The additional imports (onBeforeUnmount and watch) are correctly added to support the reactive interval lifecycle management.


34-46: LGTM!

The relativeReactive prop is well-documented with clear JSDoc comments, and the default value of true maintains backward compatibility with the previous behaviour where the interval always ran.


67-83: Excellent implementation with proper resource management!

The reactive interval lifecycle is correctly implemented:

  • The watch with immediate: true ensures the interval starts/stops appropriately based on the relativeReactive prop value
  • Interval cleanup is properly handled both on prop changes and component unmount
  • This also fixes a memory leak in the previous implementation where setInterval was called without any cleanup
test/nuxt/nuxt-time.test.ts (2)

81-123: Excellent test coverage!

Both test cases properly verify the relativeReactive prop behaviour:

  • The first test confirms that the interval updates the displayed time when relativeReactive is true
  • The second test confirms that the time remains static when relativeReactive is false

The 1-second delay is appropriate for testing the interval behaviour.


163-207: LGTM!

The SSR hydration test updates are correct:

  • The addition of relativeStyle: 'short' explains the changes to shortened format ("2 mo ago", "1 yr ago")
  • The expected HTML correctly includes the new data-relative-reactive="true" attribute alongside data-relative-style="short"
  • These changes properly verify that the new prop is serialised for SSR hydration

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 24, 2025

Open in StackBlitz

@nuxt/kit

npm i https://pkg.pr.new/@nuxt/kit@33559

nuxt

npm i https://pkg.pr.new/nuxt@33559

@nuxt/rspack-builder

npm i https://pkg.pr.new/@nuxt/rspack-builder@33559

@nuxt/schema

npm i https://pkg.pr.new/@nuxt/schema@33559

@nuxt/vite-builder

npm i https://pkg.pr.new/@nuxt/vite-builder@33559

@nuxt/webpack-builder

npm i https://pkg.pr.new/@nuxt/webpack-builder@33559

commit: a9545b4

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 24, 2025

CodSpeed Performance Report

Merging #33559 will not alter performance

Comparing NamesMT:feat/relativeReactive (a9545b4) with main (56a257a)

Summary

✅ 10 untouched

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant