fix(nuxt): use correct unit thresholds for relative time #32893
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #32893 will not alter performanceComparing Summary
|
Updated relative date formatting logic to switch units at correct boundaries: - Hours now switch to days at 24 hours (instead of 60) - Explicit thresholds defined for each unit - Kept Infinity as the upper bound for years Prevents cases like "25 hours ago" showing instead of "1 day ago".
WalkthroughThe changes introduce a custom Vite plugin within the Nuxt configuration to modify tree-shaking and SSR composable handling. The plugin transforms source code at build time, ensuring the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/nuxt/src/app/components/nuxt-time.vue (1)
73-92: Unit-threshold selection logic looks correct; consider rounding nuanceThe thresholds map aligns with the intended behaviour (60s, 60m, 24h, ~30d, 12m, ∞y). One small nit: rounding can occasionally show “60 seconds” at ~59.5s. If you want to avoid that, round toward zero (or floor/ceil by sign) before formatting.
- const value = diffInSeconds / seconds - return (formatter.value as Intl.RelativeTimeFormat).format(Math.round(value), unit) + const value = diffInSeconds / seconds + const rounded = value < 0 ? Math.ceil(value) : Math.floor(value) + return (formatter.value as Intl.RelativeTimeFormat).format(rounded, unit)Optional: If you prefer rounding to nearest, keep as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
nuxt.config.ts(2 hunks)packages/nuxt/src/app/components/nuxt-time.vue(2 hunks)test/nuxt/nuxt-time.test.ts(2 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.tsnuxt.config.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 (5)
📓 Common learnings
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
📚 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 **/*.{test,spec}.{ts,tsx,js,jsx} : Write unit tests for core functionality using `vitest`
Applied to files:
test/nuxt/nuxt-time.test.ts
📚 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 **/e2e/**/*.{ts,js} : Write end-to-end tests using Playwright and `nuxt/test-utils`
Applied to files:
test/nuxt/nuxt-time.test.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
nuxt.config.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
PR: nuxt/nuxt#29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
nuxt.config.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: codeql (javascript-typescript)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (3)
nuxt.config.ts (1)
24-41: Enhance transform robustness and restrict to test environmentTo avoid brittle string matches and unintended production changes:
- Wrap the Vite plugin registration so it only runs in test mode (e.g.
process.env.VITESTorNODE_ENV === 'test').- Use regular expressions that tolerate varying whitespace when stripping the
onPrehydrateguard and replacing the server‐only guard.- Verify whether
useAttrsis auto-imported in your setup. If not, injectimport { useAttrs } from 'vue'into the<script setup>block of the SFC.Suggested diff:
- addVitePlugin(() => ({ + // only for tests + if (!(process.env.VITEST || process.env.NODE_ENV === 'test')) { + return + } + addVitePlugin(() => ({ name: 'preserve-ssr-composables', enforce: 'pre', transform(code, id) { let replaced = false if (code.includes('export function onPrehydrate')) { replaced = true - code = code.replaceAll('if (import.meta.client) { return }', '') + // tolerate varying whitespace + code = code.replace(/\bif\s*\(\s*import\.meta\.client\s*\)\s*\{\s*return\s*\}/g, '') } if (id.includes('nuxt-time.vue')) { replaced = true - code = code.replace('if (import.meta.server) {', 'if (useAttrs().ssr) {') + // replace the first server-only guard, tolerant to spacing + code = code.replace(/\bif\s*\(\s*import\.meta\.server\s*\)\s*\{/, + 'if (useAttrs().ssr) {') } if (replaced) { return code } }, }))Please confirm that
useAttrsis available via auto-imports in this Nuxt config; otherwise, inject the Vue import into the SFC.test/nuxt/nuxt-time.test.ts (1)
4-4: LGTM: head access added for prehydration assertionsImporting
injectHeadis appropriate for validating the injected script and hydration flow.packages/nuxt/src/app/components/nuxt-time.vue (1)
69-71: Early return for non-relative path is clear and efficientGood simplification; avoids unnecessary branching later.
| nuxt.options.optimization.treeShake.composables.client ||= {} | ||
| nuxt.options.optimization.treeShake.composables.client['#app'] = nuxt.options.optimization.treeShake.composables.client['#app']?.filter(c => c !== 'onPrehydrate') || [] | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Guard tree-shake override to test-only and ensure safety on missing paths
This mutation currently runs unconditionally. Restrict to test runs to avoid impacting dev/prod builds, and guard the nested path in case optimization is undefined in some environments.
- // this preserves onPrehydrate so we can make assertions on the client
- nuxt.options.optimization.treeShake.composables.client ||= {}
- nuxt.options.optimization.treeShake.composables.client['#app'] = nuxt.options.optimization.treeShake.composables.client['#app']?.filter(c => c !== 'onPrehydrate') || []
+ // this preserves onPrehydrate so we can make assertions on the client (test-only)
+ if (process.env.VITEST || process.env.NODE_ENV === 'test') {
+ nuxt.options.optimization ??= {} as any
+ nuxt.options.optimization.treeShake ??= {} as any
+ nuxt.options.optimization.treeShake.composables ??= {} as any
+ nuxt.options.optimization.treeShake.composables.client ??= {}
+ nuxt.options.optimization.treeShake.composables.client['#app'] =
+ nuxt.options.optimization.treeShake.composables.client['#app']?.filter(c => c !== 'onPrehydrate') || []
+ }Action: Please confirm this module only runs in tests (or wrap the entire module body with the same guard).
Guard test-only tree-shake override & safe-guard nested options
To avoid impacting dev/prod builds and prevent runtime errors if nested properties are missing, please:
- Wrap the override so it only runs in a test environment (
process.env.VITEST || process.env.NODE_ENV === 'test'). - Nullish-assign at each level (
optimization,treeShake,composables,composables.client) before filtering outonPrehydrate.
Proposed diff in nuxt.config.ts:
- // this preserves onPrehydrate so we can make assertions on the client
- nuxt.options.optimization.treeShake.composables.client ||= {}
- nuxt.options.optimization.treeShake.composables.client['#app'] =
- nuxt.options.optimization.treeShake.composables.client['#app']?.filter(c => c !== 'onPrehydrate') || []
+ // test-only: preserve onPrehydrate for client assertions
+ if (process.env.VITEST || process.env.NODE_ENV === 'test') {
+ nuxt.options.optimization ??= {} as any
+ nuxt.options.optimization.treeShake ??= {} as any
+ nuxt.options.optimization.treeShake.composables ??= {} as any
+ nuxt.options.optimization.treeShake.composables.client ??= {}
+ nuxt.options.optimization.treeShake.composables.client['#app'] =
+ nuxt.options.optimization.treeShake.composables.client['#app']
+ ?.filter(c => c !== 'onPrehydrate') || []
+ }Action: Confirm this code block only needs to run in tests, or apply the same guard around the entire module registration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| nuxt.options.optimization.treeShake.composables.client ||= {} | |
| nuxt.options.optimization.treeShake.composables.client['#app'] = nuxt.options.optimization.treeShake.composables.client['#app']?.filter(c => c !== 'onPrehydrate') || [] | |
| // test-only: preserve onPrehydrate for client assertions | |
| if (process.env.VITEST || process.env.NODE_ENV === 'test') { | |
| nuxt.options.optimization ??= {} as any | |
| nuxt.options.optimization.treeShake ??= {} as any | |
| nuxt.options.optimization.treeShake.composables??= {} as any | |
| nuxt.options.optimization.treeShake.composables.client ??= {} | |
| nuxt.options.optimization.treeShake.composables.client['#app'] = | |
| nuxt.options.optimization.treeShake.composables.client['#app'] | |
| ?.filter(c => c !== 'onPrehydrate') || [] | |
| } |
🤖 Prompt for AI Agents
In nuxt.config.ts around lines 21 to 23, the code modifies nested properties
without checking if they exist and runs unconditionally, which can cause runtime
errors and affect non-test environments. To fix this, wrap the entire block in a
condition that checks if the environment is a test environment
(process.env.VITEST or process.env.NODE_ENV === 'test'), and use nullish
assignment (??=) at each nested level (optimization, treeShake, composables,
composables.client) before accessing or modifying properties to ensure they
exist safely.
🔗 Linked issue
No existing issue
📚 Description
This PR refines the relative time formatting logic by introducing explicit, accurate thresholds for all time units to decide when to switch to the next larger unit.
Issue:
The previous implementation used a uniform threshold of 60 for all units (seconds, minutes, hours, days, etc.). This approach caused unnatural outputs such as:
"25 hours ago"instead of"1 day ago""45 days ago"instead of"1 month ago""15 months ago"instead of"1 year ago"Because it treated hours, days, and months as if they rolled over at 60, which is only true for seconds-to-minutes and minutes-to-hours.
What changed:
Added unit-specific thresholds to define when to switch units:
Example outputs before fix:
Example outputs after fix: