-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat(nuxt): NuxtTime: add relativeReactive prop
#33559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
WalkthroughA new boolean prop Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
There was a problem hiding this 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-reactiveprop and its usage. The example effectively demonstrates disabling automatic updates.For improved discoverability, consider adding a dedicated subsection for
relative-reactiveunder "Relative Time Formatting Props" (similar to howdatetimeandlocalehave dedicated sections), rather than only documenting it inline within therelativeexample. This would follow the established pattern and make the prop more discoverable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 (
onBeforeUnmountandwatch) are correctly added to support the reactive interval lifecycle management.
34-46: LGTM!The
relativeReactiveprop is well-documented with clear JSDoc comments, and the default value oftruemaintains 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
watchwithimmediate: trueensures the interval starts/stops appropriately based on therelativeReactiveprop value- Interval cleanup is properly handled both on prop changes and component unmount
- This also fixes a memory leak in the previous implementation where
setIntervalwas called without any cleanuptest/nuxt/nuxt-time.test.ts (2)
81-123: Excellent test coverage!Both test cases properly verify the
relativeReactiveprop behaviour:
- The first test confirms that the interval updates the displayed time when
relativeReactiveistrue- The second test confirms that the time remains static when
relativeReactiveisfalseThe 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 alongsidedata-relative-style="short"- These changes properly verify that the new prop is serialised for SSR hydration
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33559 will not alter performanceComparing Summary
|
🔗 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
relativeReactiveprop to enable/disable the interval clock forNuxtTimerelative render, supporting dynamic toggling of the clock, adding and updating tests, and note it to the docs.