fix(nuxt): use greedy catchall when /index is the last segment#31528
fix(nuxt): use greedy catchall when /index is the last segment#31528
/index is the last segment#31528Conversation
|
|
WalkthroughThe changes update the route generation logic and extend the dynamic route configurations within the project. In the utility file, the condition for appending a subsequent segment to the route path now includes an additional check to ensure the segment is not equal to "index". This modification refines how route paths are constructed based on file segment names. In the test file, a new dynamic route is introduced, mapping the ✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/nuxt/src/pages/utils.ts (1)
142-142: Consider adding a comment to clarify this condition's purposeThis condition now checks that the next segment is not 'index' to determine whether to use a greedy catchall pattern. Adding a comment would help future developers understand why this special case exists.
- const routePath = getRoutePath(tokens, segments[i + 1] !== undefined && segments[i + 1] !== 'index') + // Use greedy catchall pattern when `/index` is the last segment + const hasSucceedingNonIndexSegment = segments[i + 1] !== undefined && segments[i + 1] !== 'index' + const routePath = getRoutePath(tokens, hasSucceedingNonIndexSegment)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/nuxt/test/__snapshots__/pages-override-meta-disabled.test.ts.snapis excluded by!**/*.snappackages/nuxt/test/__snapshots__/pages-override-meta-enabled.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
packages/nuxt/src/pages/utils.ts(1 hunks)packages/nuxt/test/pages.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (2)
packages/nuxt/test/pages.test.ts (2)
611-613: Good test case for the new functionalityThis test case properly covers the specific scenario where a catchall route has an index file as the last segment, which is exactly what the PR aims to fix.
616-622: Correct expected output with greedy catchall patternThe expected output correctly verifies that the greedy catchall pattern (
/:id(.*)*) is used when/indexis the last segment in a catchall route path, which aligns with the PR objective.
@nuxt/kit
nuxt
@nuxt/schema
@nuxt/rspack-builder
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #31528 will not alter performanceComparing Summary
|
|
@danielroe Thank you for the fix! I have a couple of questions regarding the availability of these changes:
This would be really helpful for our current development process, as we're actively working with dynamic routes and i18n. |
|
you can expect a patch release this weekend, and before then (once this is cherry picked to 3.x) you can use the nightly channel: https://nuxt.com/docs/guide/going-further/nightly-release-channel |
🔗 Linked issue
resolves nuxt-modules/i18n#3464
📚 Description
we don't want to apply the more narrow regexp when there is no following segment, but we neglected to check for
/indexas a segment (which we ignore) meaning the path matcher we created had too low of a priority