Skip to content

fix: prevent var name clash in generated route for escaped strings - #6680

Merged
schiller-manuel merged 1 commit into
mainfrom
generator-escaped-var-names
Feb 16, 2026
Merged

fix: prevent var name clash in generated route for escaped strings#6680
schiller-manuel merged 1 commit into
mainfrom
generator-escaped-var-names

Conversation

@schiller-manuel

@schiller-manuelschiller-manuel commented Feb 16, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Release Notes

  • New Features

    • Added home route navigation support at the root path.
    • Enhanced handling of routes with special characters and custom token escaping.
  • Tests

    • Added test coverage for home route rendering and client-side navigation functionality.

@coderabbitai

coderabbitaiBot commented Feb 16, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The PR extends route path escaping in the router generator to properly handle bracket-wrapped route segments. When a route segment is fully bracket-escaped (e.g., [index]), the generator now uses the original bracket-wrapped path when determining variable names to prevent collisions. Updates include regenerated route definitions, new E2E route handlers, and test coverage for the new behavior.

Changes

Cohort / File(s)Summary
E2E Route Type Definitions
e2e/react-router/escaped-special-strings/src/routeTree.gen.ts
Updated route imports and constants from generic names (IndexRoute, RouteRoute, LazyRoute) to escaped-variant identifiers (Char91indexChar93Route, Char91routeChar93Route, Char91lazyChar93Route). Added new root path ("/") route mapping and updated type declarations in FileRoutesByFullPath, FileRoutesByTo, and FileRoutesById interfaces.
E2E Route Implementation
e2e/react-router/escaped-special-strings/src/routes/index.tsx
New file defining the root path ("/") route using createFileRoute, with a HomeComponent rendering a home page with title, path, and description text.
E2E Navigation Updates
e2e/react-router/escaped-special-strings/src/routes/__root.tsx
Added a new navigation link to the root path ("/") labeled "/ (home)" with test identifier "link-home" in the root component's nav.
E2E Test Suite
e2e/react-router/escaped-special-strings/tests/escaped-routes.spec.ts
Added tests verifying the home index route renders at "/", and client-side navigation from /index to the home route with correct URL and title updates.
Router Generator Core Logic
packages/router-generator/src/filesystem/physical/getRouteNodes.ts
Modified getRouteMeta to conditionally select variableName based on whether any segment in the original bracket-escaped route path is fully bracket-wrapped, using originalRoutePath for fully escaped segments.
Router Generator Snapshots
packages/router-generator/tests/generator/escaped-custom-tokens/routeTree.snapshot.ts, packages/router-generator/tests/generator/escaped-special-strings/routeTree.snapshot.ts
Updated route type references and variable names from token-escaped forms to new escaped-safe identifiers (e.g., Char91indexChar93Route, NestedChar91_1nd3xChar93Route). Regenerated interface mappings and module augmentations across all affected route definitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

package: router-generator, package: react-router

Suggested reviewers

  • nlynzaad

Poem

🐰 A winding path through brackets wrapped so tight,
Route names escape to variables' delight!
Collisions avoided with a clever twist,
The home route blooms—no escaping this bliss!

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately describes the main change: fixing variable name clashes in generated routes for escaped strings by renaming identifiers to use bracket-escaped forms (e.g., Char91indexChar93Route).
Merge Conflict Detection✅ Passed✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch generator-escaped-var-names

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

@nx-cloud

nx-cloudBot commented Feb 16, 2026

Copy link
Copy Markdown
Contributor

View your CI Pipeline Execution ↗ for commit ecdd8c4

CommandStatusDurationResult
nx affected --targets=test:eslint,test:unit,tes...✅ Succeeded10m 53sView ↗
nx run-many --target=build --exclude=examples/*...✅ Succeeded33sView ↗

☁️ Nx Cloud last updated this comment at 2026-02-16 18:10:06 UTC

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/router-generator/src/filesystem/physical/getRouteNodes.ts (1)

449-461: Align escaped-segment detection with the comment examples.
The predicate only matches segments fully wrapped in brackets, so [_]auth in the example won’t trigger. Either broaden the check to include partial bracket escapes or narrow the example to match current behavior.

✏️ Suggested comment tweak
- // bracket-wrapped (e.g. [index], [route], [_]auth) to avoid collisions+ // bracket-wrapped (e.g. [index], [route]) to avoid collisions

@schiller-manuel
schiller-manuel merged commit 227dfaf into mainFeb 16, 2026
4 of 5 checks passed
@schiller-manuel
schiller-manuel deleted the generator-escaped-var-names branch February 16, 2026 18:10
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@schiller-manuel