Skip to content

fix(build): escape dynamic route segments in chunk names - #4508

Open
RezaRahemtola wants to merge 1 commit into
nitrojs:mainfrom
RezaRahemtola:fix/build-escape-dynamic-route-chunk-names
Open

fix(build): escape dynamic route segments in chunk names#4508
RezaRahemtola wants to merge 1 commit into
nitrojs:mainfrom
RezaRahemtola:fix/build-escape-dynamic-route-chunk-names

Conversation

@RezaRahemtola

Copy link
Copy Markdown

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

getChunkName returns _routes/<path>.mjs for route handlers, and routeToFsPath converts dynamic segments like :id to [id]. Rollup rejects [id] as an invalid chunkFileNames placeholder, so building with the Rollup builder fails on nested dynamic routes like server/api/applications/[id]/context.get.ts.

It looks like Rolldown (the default builder) tolerates the brackets, which is why this slipped through.

This PR encodes brackets ([ and ]) as %5B/%5D in the chunk name so it's a valid chunkFileNames pattern.
This also avoids any collision with literal route segments named _id_ (the concern that closed the similar PR I found #4393).
Also updated the unit test and added a nested dynamic route case.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@RezaRahemtola
RezaRahemtola requested a review from pi0 as a code owner August 5, 2026 01:06
@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

@RezaRahemtola is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 20ed5f63-7c38-47aa-968d-9701c7c4dea2

📥 Commits

Reviewing files that changed from the base of the PR and between 52abde8 and a1e3626.

📒 Files selected for processing (2)
  • src/build/chunks.ts
  • test/unit/chunks.test.ts

📝 Walkthrough

Walkthrough

Changes

Route chunk encoding

Layer / File(s) Summary
Encode dynamic route chunks
src/build/chunks.ts, test/unit/chunks.test.ts
Route-derived chunk filenames percent-encode [ and ]. Tests cover direct and nested dynamic routes.
Estimated code review effort: 2 (Simple) ~10 minutes

Possibly related PRs

  • nitrojs/nitro#4027: Both changes update route filesystem path handling in src/build/chunks.ts.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits syntax and accurately describes the dynamic route chunk-name fix.
Description check ✅ Passed The description clearly explains the Rollup failure, bracket encoding fix, test updates, and collision avoidance.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant