feat(nuxt): migrate to unrouting for nuxt file system -> route generation#34316
feat(nuxt): migrate to unrouting for nuxt file system -> route generation#34316
Conversation
|
|
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
Merging this PR will degrade performance by 36.29%
Performance Changes
Comparing Footnotes
|
- Add PagesContext with persistent route tree for dev-mode incremental updates - Use unrouting's addFile/removeFile instead of full tree rebuild on file add/unlink - Add picomatch for compiled glob pattern matching on watcher events - Extract augmentAndResolve for shared downstream pipeline - Remove redundant uniqueBy deduplication (handled by unrouting tree) - Add branch-agnostic dev server lifecycle benchmark
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
WalkthroughIntroduces a PagesContext-driven pipeline for Nuxt pages with persistent route-tree tracking and incremental updates in dev mode. Adds PagesContext, createPagesContext, augmentAndResolve, toNuxtPages, and picomatch-based page pattern matching. Integrates handleRouteRules and wires incremental watcher logic in the pages module to mutate the PagesContext on page file add/remove and emit updated pages, with a fallback to full rebuilds when incremental updates are not applicable or fail. Tests updated to use the new PagesContext API and revised generateRoutesFromFiles signature. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/nuxt/src/pages/utils.ts`:
- Around line 89-110: The mode-detection in toNuxtPages incorrectly prioritises
'server' when both modes are present; update the logic in the toNuxtPages
function so that you only set page.mode when route.modes exists and contains
exactly one entry: if route.modes has length === 1 and that single value is
'server' set page.mode = 'server'; if length === 1 and the value is 'client' set
page.mode = 'client'; otherwise (both present, empty or undefined) leave
page.mode undefined to keep the page isomorphic. Use the existing route.modes
and page.mode symbols to locate and change the conditional.
🧹 Nitpick comments (4)
packages/nuxt/test/pages-dev.bench.ts (2)
50-50: Consider usingPagesContextOptionsdirectly instead of a local type alias.
ContextOptionsduplicates a subset ofPagesContextOptionsfromutils.ts. Using the existing type would reduce duplication and stay in sync with any future additions.♻️ Suggested change
-type ContextOptions = { roots: string[] } +import type { PagesContextOptions } from '../src/pages/utils.ts'Then replace
ContextOptionswithPagesContextOptionsin thecreateSimulatorsignature.
63-66: Unnecessary alias forcreatePagesContext.
createCtxis assignedcreatePagesContextbut never reassigned or swapped — it just adds indirection.♻️ Suggested change
-function createSimulator (filePaths: string[], opts: ContextOptions): DevSimulator { - const createCtx = createPagesContext - let ctx = createCtx(opts) +function createSimulator (filePaths: string[], opts: ContextOptions): DevSimulator { + let ctx = createPagesContext(opts) const files = filePaths.map(p => ({ path: p }))And update line 70:
- ctx = createCtx(opts) + ctx = createPagesContext(opts)packages/nuxt/src/pages/module.ts (1)
110-121: Consider consolidatingresolvePagesRoutesandaugmentAndResolvePageswrappers.Both wrappers add
handleRouteRulesafter calling the underlying utility. IfhandleRouteRuleswere folded intoaugmentAndResolveinutils.ts, these two separate wrappers could be simplified. That said, keeping route-rule handling in the module layer (rather than utils) is a reasonable separation of concerns — this is just a thought for future consideration.packages/nuxt/src/pages/utils.ts (1)
89-96:toNuxtPagesis not exported — verify this is intentional.The function declaration at line 89 uses
function toNuxtPages(...)withoutexport. The AI summary mentions it as a new export, and the module import line (module.ts line 19) does not import it, so internal-only usage is likely fine. However, if external consumers (e.g. module authors) need to convert unrouting output, you may want to export it in the future.
packages/nuxt/src/pages/utils.ts
Outdated
| function toNuxtPages (routes: VueRoute[]): NuxtPage[] { | ||
| return routes.map((route) => { | ||
| const page: NuxtPage = { | ||
| name: route.name, | ||
| path: route.path, | ||
| file: route.file, | ||
| children: route.children?.length ? toNuxtPages(route.children) : [], | ||
| } | ||
|
|
||
| if (route.modes?.includes('server')) { | ||
| page.mode = 'server' | ||
| } else if (route.modes?.includes('client')) { | ||
| page.mode = 'client' | ||
| } | ||
|
|
||
| if (route.meta?.groups) { | ||
| page.meta = { ...page.meta, groups: route.meta.groups } | ||
| } | ||
|
|
||
| return page | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for VueRoute type definition
rg "type VueRoute|interface VueRoute" --type ts --type tsx -A 10Repository: nuxt/nuxt
Length of output: 80
🏁 Script executed:
# Check for mode-related tests or examples
rg "modes.*server.*client|mode.*server" packages/nuxt/src/pages --type ts --type tsx -B 2 -A 2Repository: nuxt/nuxt
Length of output: 80
🏁 Script executed:
# Examine the utils.ts file more broadly to understand context
cat -n packages/nuxt/src/pages/utils.ts | head -120Repository: nuxt/nuxt
Length of output: 5318
🏁 Script executed:
# Search for unrouting imports and usage
rg "unrouting|VueRoute" packages/nuxt/src/pages --type ts --type tsx | head -20Repository: nuxt/nuxt
Length of output: 80
🏁 Script executed:
# Search for tests related to toNuxtPages or mode handling
fd ".*\.test\.|.*\.spec\." packages/nuxt/src/pages --type fRepository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Look for any test files mentioning modes or toNuxtPages
rg "toNuxtPages|modes.*server.*client" packages/nuxt/src/pages --type ts -B 3 -A 3Repository: nuxt/nuxt
Length of output: 1905
🌐 Web query:
unrouting npm package VueRoute modes property documentation
💡 Result:
In unrouting, “modes” are file-suffix variants (e.g. page.client.vue, page.server.vue) that the parser/tree builder can detect, and that the Vue Router emitter can surface on the emitted route objects.
- Configure detection: pass
modes: string[]tobuildTree(...)(it extendsParsePathOptions) to tellunroutingwhich suffixes to treat as modes (example:['client', 'server']). When present, detected modes are recorded. [1] - Low-level parse output:
parsePath(...)returnsParsedPathobjects whosemetacan includemodes?: string[]. [2] - Vue Router emit output (
VueRoute):toVueRouter4(...)returnsVueRoute[], where eachVueRoutemay includemodes?: string[]described as “Mode variants” (only present when mode files exist). [2]
References
[1] unjs/unrouting README — buildTree options include modes (mode suffixes to detect). (github.com)
[2] unjs/unrouting README — VueRoute interface includes modes?: string[]; ParsedPath.meta.modes?: string[]. (github.com)
Citations:
🏁 Script executed:
# Find NuxtPage type definition
rg "interface NuxtPage|type NuxtPage" --type ts -A 15Repository: nuxt/nuxt
Length of output: 3720
🏁 Script executed:
# Search for any tests or examples using .client.vue or .server.vue
rg "\.client\.vue|\.server\.vue" packages/nuxt --type ts -B 2 -A 2 | head -40Repository: nuxt/nuxt
Length of output: 2392
🏁 Script executed:
# Look for any documentation or comments about mode property
rg "mode.*server|mode.*client" packages/nuxt/src/pages --type ts -B 2 -A 2Repository: nuxt/nuxt
Length of output: 3480
🌐 Web query:
unrouting VueRoute modes when both client and server variants exist documentation
💡 Result:
When you have both a client (browser) and server (SSR/Node) router, Vue Router’s docs recommend using different “history” implementations per environment:
- Client (browser): use HTML5 history for clean URLs:
createWebHistory()(or hash if you can’t configure the server). (router.vuejs.org) - Server (SSR/Node): use memory/abstract history because there is no real browser URL/history:
- Vue Router v4:
createMemoryHistory()is “perfect for Node environment and SSR”. (router.vuejs.org) - Vue Router v3: router “mode” becomes
abstractin non-browser environments (Node). (v3.router.vuejs.org)
- Vue Router v4:
Key implication (the “unrouting/404 on refresh” issue):
- If you use HTML5 history on the client, you must configure the server to return your app entry (e.g.,
index.html) for unknown paths, otherwise direct loads like/user/123will 404. (router.vuejs.org)
Nuxt note (if that’s what you mean by client/server variants):
- Nuxt 3’s
router.options.hashModeis specifically for SPA mode, and SSR is not supported with hash mode. (dev.nuxt.com) - Nuxt 2 documents that changing router
modeis not recommended due to SSR. (v2.nuxt.com)
Citations:
- 1: https://router.vuejs.org/guide/essentials/history-mode?utm_source=openai
- 2: https://router.vuejs.org/guide/essentials/history-mode?utm_source=openai
- 3: https://v3.router.vuejs.org/api/?utm_source=openai
- 4: https://router.vuejs.org/guide/essentials/history-mode?utm_source=openai
- 5: https://dev.nuxt.com/docs/3.x/api/configuration/nuxt-config?utm_source=openai
- 6: https://v2.nuxt.com/api/configuration-router?utm_source=openai
🏁 Script executed:
# Search for test files related to pages routes and modes
fd ".*\.test\.|.*\.spec\." packages/nuxt/src/pages | head -20Repository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Look for any existing tests for toNuxtPages function
rg "toNuxtPages" packages/nuxt --type ts -B 5 -A 10Repository: nuxt/nuxt
Length of output: 2510
🏁 Script executed:
# Check if there are any discussions or examples of .server.vue + .client.vue together
rg "\.server\.vue.*\.client\.vue|\.client\.vue.*\.server\.vue" packages/nuxt --type tsRepository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Search for any test fixtures with dual-mode pages
fd "page\.(server|client)\.vue" packages/nuxt/test --type fRepository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Search for unrouting documentation or issues discussing dual-mode handling
rg "modes.*server.*client|server\.vue.*client\.vue" packages/nuxt --type ts -B 3 -A 3Repository: nuxt/nuxt
Length of output: 605
🏁 Script executed:
# Check the schema definition more carefully for NuxtPage.mode
cat -n packages/schema/src/types/hooks.ts | sed -n '1,150p'Repository: nuxt/nuxt
Length of output: 5893
🌐 Web query:
unrouting component islands dual mode page both server and client variants
💡 Result:
In unjs/unrouting, “dual mode” (both server and client variants for the same route) is handled via mode variant support: you can create multiple files that map to the same URL by adding a configurable mode suffix such as .client and .server (e.g., page.client.vue, page.server.vue). During tree-building you enable detection via the modes option (e.g., modes: ['client', 'server']). When emitting to Vue Router 4, the resulting route record can include a modes?: string[] field indicating which variants exist for that route. [1]
This feature is about filesystem routing + emitting route definitions/metadata (including “named view” support like index@sidebar.vue), not a full “islands” rendering system by itself. [1]
Sources:
[1] unjs/unrouting README (mode variants, modes option, Vue Router emit shape) (github.com)
Citations:
Fix mode-detection logic to handle dual-mode pages correctly.
When route.modes contains both 'server' and 'client' (indicating both .server.vue and .client.vue variants exist), the route should remain isomorphic without a mode set, not be forced to mode: 'server'. The current logic at lines 98–102 prioritises 'server' unconditionally, which incorrectly converts dual-mode pages into server-only islands.
Update the mode assignment to:
modescontains only'server'→ setmode: 'server'modescontains only'client'→ setmode: 'client'modescontains both or is empty/undefined → leave undefined (isomorphic)
🤖 Prompt for AI Agents
In `@packages/nuxt/src/pages/utils.ts` around lines 89 - 110, The mode-detection
in toNuxtPages incorrectly prioritises 'server' when both modes are present;
update the logic in the toNuxtPages function so that you only set page.mode when
route.modes exists and contains exactly one entry: if route.modes has length ===
1 and that single value is 'server' set page.mode = 'server'; if length === 1
and the value is 'client' set page.mode = 'client'; otherwise (both present,
empty or undefined) leave page.mode undefined to keep the page isomorphic. Use
the existing route.modes and page.mode symbols to locate and change the
conditional.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nuxt/test/pages.test.ts (1)
70-78:⚠️ Potential issue | 🟡 MinorTest files lack explicit
priorityvalues, creating inconsistency with production code.In
resolvePagesRoutes(utils.ts, lines 97–101), each layer directory receives an explicitprioritybased on its index. However, the test at lines 70–78 maps files as{ path: file.path }without settingpriority, so all files have undefined priority.The "should correctly merge nested routes" test case (line 697) does pass with correct output (confirmed by snapshots), but this works despite the omission rather than by explicit intent. Adding explicit priority values would improve consistency with production code and make layer ordering more transparent.
Suggested refinement: Derive priority explicitly from the
rootsarray index:Suggested fix
- const files = test.files.map(file => ({ path: file.path })) - - result = generateRoutesFromFiles(files, { roots: ['pages/', 'layer/pages/'] }).map((route, index) => { + const roots = ['pages/', 'layer/pages/'] + const files = test.files.map(file => { + const rootIndex = roots.findIndex(root => file.path.startsWith(root)) + return { + path: file.path, + priority: rootIndex >= 0 ? rootIndex : 0, + } + }) + + result = generateRoutesFromFiles(files, { roots }).map((route, index) => {
🧹 Nitpick comments (1)
packages/nuxt/src/pages/utils.ts (1)
63-66: Type difference betweentoVueRouter4return type and declaredemitreturn type: ensure conversion fromVueRoute[]toNuxtPage[]is intentional.
toVueRouter4(tree, emitOptions)returnsVueRoute[]from the unrouting package, whereVueRoutehasmodes?: string[]. However, theemit()method's interface declaration specifies return typeNuxtPage[], whereNuxtPagehasmode?: 'client' | 'server' | 'all'. Although TypeScript's structural typing permits this due to interface compatibility, the semantic difference betweenmodesandmodeindicates a conversion should occur. Verify that theemitOptions.attrs: { mode: modes }configuration properly converts the modes array into the singular mode property for each route before returning, or add an explicit comment clarifying why the type difference is acceptable here.
🔗 Linked issue
resolves #23521
📚 Description
this migrates nuxt to use
unrouting, which has a slightly slower initial start (6ms -> 8ms for a large app) but is much more performant for dev server changes (28x faster if you are not adding/removing pages, and ~15% faster even if you are) 🔥it's also more solid (it's not sensitive to page order) and uses a trie for constructing routes.