Skip to content

Conversation

nlynzaad
Copy link
Contributor

@nlynzaad nlynzaad commented Oct 15, 2025

when a route group is defined, the generator strips the group from the path but keeps the "/" preceding this group in the path resulting in these paths being treated as routes by the generator.

For example /abc/(group) is resolved to /abc/ instead of /abc. This results in a few issues. One being that when both route and index are defined in a group, the route is determined to have a higher rank when sorting the routes and hence is matched prior to the index.

This PR resolves this issue by checking if the route is a layout (both pathless and non-pathless) and if so removes any trailing slash that remains after the group has been split.

Due to this error paths like /(group)/_layout resulted in /(group) to be defined as a virtual route to load _layout in this virtual parent route. With this fix the _layout route is correctly loaded with the rootRoute being its parent. Due to this, the generated route trees for basic-file-based e2e tests and the snapshots for the route-group unit tests in router-generator had to be updated as well.

This PR also adds a test to the path-above-route-in-group to test for the index path being correctly handled when used together with a route path in the same group.

This resolves #5486

Summary by CodeRabbit

  • New Features
    • Group routes are now exposed directly under the root, simplifying navigation and type mappings.
    • Added support for index routes within grouped paths, improving addressability of “path above route in group” scenarios.
  • Bug Fixes
    • Normalized trailing slashes for layout and pathless layout routes to prevent duplicate or inconsistent paths.
  • Tests
    • Updated e2e and snapshot tests to reflect the new routing structure and type surfaces.

Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

Walkthrough

Refactors generated route trees to expose group-related routes directly under the root, removes intermediate group route nodes, adds trailing-slash normalization for layout routes in the generator, and introduces an index route for a path-above-route-in-group scenario. Updates associated snapshots for React and Solid e2e and generator tests.

Changes

Cohort / File(s) Summary
React Router e2e route tree
e2e/react-router/basic-file-based/src/routeTree.gen.ts
Reparent group routes to rootRouteImport; remove groupRoute/groupRouteWithChildren; update FileRoutesBy* mappings so '/' maps to IndexRoute; add groupLayoutRoute, groupInsideRoute, groupLazyinsideRoute, groupSubfolderInsideRoute.
Solid Router e2e route tree
e2e/solid-router/basic-file-based/src/routeTree.gen.ts
Same structural reparenting as React; expose group-prefixed routes at root; remove /(group) entries; update FileRoutesBy* typings; '/' maps to IndexRoute.
Router generator core
packages/router-generator/src/generator.ts
Add removeTrailingSlash usage for layout and pathless_layout nodes to normalize cleanedPath; import utility; adjust path cleaning in two spots.
Generator tests — nested groups with layouts
packages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.ts
Replace group placeholders with explicit layout routes (.../_layout-*); reparent to root; update FileRoutesBy* unions; apply ._addFileTypes<FileRouteTypes>() in route tree construction.
Generator tests — path above route in group (snapshot)
packages/router-generator/tests/generator/path-above-route-in-group/routeTree.snapshot.ts
Normalize /a/$b/ to /a/$b; add ABcIndexRoute under ABcRouteRoute; update FileRoutesBy* and children typings to include index route.
Generator tests — path above route in group (new file)
packages/router-generator/tests/generator/path-above-route-in-group/routes/a/$b/(c)/index.tsx
Add index.tsx implementing Route for '/a/$b/(c)/' that renders an index component.
Generator tests — route groups
packages/router-generator/tests/generator/route-groups/routeTree.snapshot.ts
Rework group/bar routes: remove /(bar) entry, introduce /(bar)/_bar under root; adjust another-group layout paths; update FileRoutesBy* and RootRouteChildren (e.g., barBarRoute).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant FS as File system routes
  participant Gen as Router Generator
  participant Utils as removeTrailingSlash
  participant RT as Generated Route Tree

  FS->>Gen: Provide nodes (including group and layout nodes)
  Note over Gen: Handle node paths and types
  Gen->>Gen: Compute cleanedPath
  alt node._fsRouteType in {layout, pathless_layout}
    Gen->>Utils: removeTrailingSlash(cleanedPath)
    Utils-->>Gen: cleanedPath (no trailing slash)
  end
  Gen->>Gen: Assign parentRoute = root for group-derived routes
  Gen->>RT: Emit routes under root (groupLayout/inside/lazyinside/subfolder)
  Note over RT: Index routes attached where path-above-group needs index
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

package: react-router, package: solid-router

Suggested reviewers

  • schiller-manuel

Poem

A rabbit thumps the routes in place,
Snips trailing slashes with tidy grace.
Groups hop home to the root so neat,
Index burrows find their seat.
Snapshots bloom like fields anew—
Paths aligned, the garden grew.
(_/)\u2003✨ Route-cheers to you!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning Several snapshot updates introduce features not mentioned in the PR objectives, such as the addition of a . _addFileTypes<FileRouteTypes>() extension and a broad restructuring of route-groups tests that go beyond just removing trailing slashes. These modifications fall outside the scope of fixing pathless route group slash normalization and correcting index rendering. Including them here risks obscuring the targeted fix and conflating unrelated changes. Isolate and remove unrelated snapshot or API changes—such as the file-types extension and extensive route-groups refactors—and submit them in a separate PR so this change can focus solely on trailing-slash normalization.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the main change, which is a fix in the router-generator to remove trailing slashes from pathless route group paths. It is clear, concise, and directly related to the modifications made in the generator code. The conventional commit prefix correctly scopes the change to the router-generator. This title gives a teammate enough context to understand the focus of the PR at a glance.
Linked Issues Check ✅ Passed The PR implements the removal of trailing slashes for layout and pathless layout routes, directly addressing the bug described in issue #5486 where index routes in dynamic grouped layouts fail to render. The generator code adds removeTrailingSlash logic and the E2E and unit tests have been updated or added to verify that index pages now load correctly at group base paths. These changes align with the expected behavior and reproduction steps from the linked issue.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ 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 index-dynamic-grouped

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4303a8a and 8835eac.

📒 Files selected for processing (7)
  • e2e/react-router/basic-file-based/src/routeTree.gen.ts (8 hunks)
  • e2e/solid-router/basic-file-based/src/routeTree.gen.ts (8 hunks)
  • packages/router-generator/src/generator.ts (2 hunks)
  • packages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.ts (5 hunks)
  • packages/router-generator/tests/generator/path-above-route-in-group/routeTree.snapshot.ts (4 hunks)
  • packages/router-generator/tests/generator/path-above-route-in-group/routes/a/$b/(c)/index.tsx (1 hunks)
  • packages/router-generator/tests/generator/route-groups/routeTree.snapshot.ts (8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript in strict mode with extensive type safety across the codebase

Files:

  • packages/router-generator/src/generator.ts
  • packages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.ts
  • e2e/solid-router/basic-file-based/src/routeTree.gen.ts
  • packages/router-generator/tests/generator/route-groups/routeTree.snapshot.ts
  • e2e/react-router/basic-file-based/src/routeTree.gen.ts
  • packages/router-generator/tests/generator/path-above-route-in-group/routes/a/$b/(c)/index.tsx
  • packages/router-generator/tests/generator/path-above-route-in-group/routeTree.snapshot.ts
packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/**

📄 CodeRabbit inference engine (AGENTS.md)

Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories

Files:

  • packages/router-generator/src/generator.ts
  • packages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.ts
  • packages/router-generator/tests/generator/route-groups/routeTree.snapshot.ts
  • packages/router-generator/tests/generator/path-above-route-in-group/routes/a/$b/(c)/index.tsx
  • packages/router-generator/tests/generator/path-above-route-in-group/routeTree.snapshot.ts
e2e/**

📄 CodeRabbit inference engine (AGENTS.md)

Store end-to-end tests under the e2e/ directory

Files:

  • e2e/solid-router/basic-file-based/src/routeTree.gen.ts
  • e2e/react-router/basic-file-based/src/routeTree.gen.ts
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

  • packages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.ts
  • packages/router-generator/tests/generator/route-groups/routeTree.snapshot.ts
  • packages/router-generator/tests/generator/path-above-route-in-group/routeTree.snapshot.ts
🧬 Code graph analysis (4)
packages/router-generator/src/generator.ts (3)
packages/router-core/src/path.ts (1)
  • removeTrailingSlash (52-57)
packages/router-generator/src/utils.ts (1)
  • removeTrailingSlash (51-53)
packages/router-generator/src/index.ts (1)
  • removeTrailingSlash (19-19)
e2e/solid-router/basic-file-based/src/routeTree.gen.ts (1)
e2e/react-router/js-only-file-based/src/routeTree.gen.js (1)
  • IndexRoute (30-34)
e2e/react-router/basic-file-based/src/routeTree.gen.ts (1)
e2e/react-router/js-only-file-based/src/routeTree.gen.js (1)
  • IndexRoute (30-34)
packages/router-generator/tests/generator/path-above-route-in-group/routeTree.snapshot.ts (1)
e2e/react-router/basic-file-based/src/routeTree.gen.ts (3)
  • FileRoutesByTo (652-730)
  • FileRoutesById (731-820)
  • FileRoutesByFullPath (566-651)
⏰ 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). (2)
  • GitHub Check: Preview
  • GitHub Check: Test
🔇 Additional comments (22)
packages/router-generator/tests/generator/path-above-route-in-group/routes/a/$b/(c)/index.tsx (1)

3-5: LGTM for index route in grouped layout

Correct use of trailing slash for index; definition is minimal and valid.

packages/router-generator/src/generator.ts (2)

36-36: Import looks good

Bringing in removeTrailingSlash from utils is appropriate for local normalization.


1227-1232: Layout/pathless layout trailing-slash normalization

This correctly removes spurious '/' left after stripping groups, fixing index routing and virtual parent creation. Note: when cleanedPath is '/', this turns it into '', which is likely desired for root layouts/pathless groups.

Please confirm this empty-string outcome is intended for root-level layouts/pathless groups and that no consumers rely on cleanedPath === '/' in those cases.

packages/router-generator/tests/generator/path-above-route-in-group/routeTree.snapshot.ts (1)

16-26: Snapshot updates align with index handling and trailing-slash fix

ABcRouteRoute path normalized ('/a/$b'), ABcIndexRoute added and wired as index. Looks consistent with the generator change.

Based on learnings

Also applies to: 32-45

packages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.ts (1)

20-31: Snapshot reflects reparented layouts and normalized paths

groupX layout routes now parented by root; index mapped to '/'. Matches generator behavior.

Based on learnings

Also applies to: 53-64, 91-95, 99-119, 189-193

e2e/react-router/basic-file-based/src/routeTree.gen.ts (1)

210-225: Generated tree matches new grouping/layout semantics

Groups are direct root children and index '/' exposure is consistent. E2E gen looks correct.

Also applies to: 382-386, 566-568, 652-654, 1079-1098

e2e/solid-router/basic-file-based/src/routeTree.gen.ts (9)

191-195: Reparenting group lazyinside route to root is correct.

IDs/paths align with group-prefixing; lazy import preserved.


197-201: Reparenting group inside route to root looks good.

Consistent ID/path mapping.


202-205: Pathless group layout normalization LGTM.

Empty path/fullPath via root parent removes the trailing slash bug; matches module augmentation below.

Please confirm e2e covers index rendering at the group base after this change.


323-326: Reparenting grouped subfolder route to root is consistent.


506-508: '/' now maps to IndexRoute.

Fixes prior accidental group intercept; good.


584-585: 'to' mapping for '/' updated to IndexRoute — consistent with FullPath map.


980-989: RootRouteChildren updated to include group routes directly under root.*

Reflects removal of virtual group parent; consistent.

Also applies to: 1891-1900


1143-1151: Module augmentation for group routes is correct.

  • lazyinside/inside parent set to root
  • '(group)/_layout' path/fullPath set to '' to avoid trailing '/'

Also applies to: 1154-1158


1318-1319: Module augmentation: grouped subfolder route parent set to root — OK.

packages/router-generator/tests/generator/route-groups/routeTree.snapshot.ts (7)

34-36: bar group layout reparented to root with empty path/fullPath — good.


58-60: '(another-group)/_layout' under fooAsdfRoute with empty path — correct.


85-85: FileRoutesByFullPath '/asdf' → fooAsdfanotherGroupLayoutRouteWithChildren — OK.


162-166: Module path for '/(bar)/_bar' set to '' with root parent — matches pathless layout normalization.


197-201: Module entry for '/(foo)/asdf/(another-group)/_layout' normalized (path: '', fullPath: '/asdf').

Prevents trailing slash; parent linkage correct.


265-266: fooAsdfRoute children updated to include normalized anotherGroup layout — consistent.

Also applies to: 273-274


283-283: RootRouteChildren includes barBarRoute — matches new structure.


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

Copy link

nx-cloud bot commented Oct 15, 2025

View your CI Pipeline Execution ↗ for commit 8835eac

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 5m 24s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 30s View ↗

☁️ Nx Cloud last updated this comment at 2025-10-15 23:03:18 UTC

Copy link

pkg-pr-new bot commented Oct 15, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5491

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5491

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5491

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5491

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5491

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5491

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5491

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5491

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5491

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5491

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5491

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5491

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5491

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5491

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5491

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5491

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5491

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5491

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5491

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5491

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5491

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5491

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5491

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5491

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5491

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5491

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5491

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5491

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5491

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5491

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5491

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5491

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5491

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5491

commit: 8835eac

@xKesvaL
Copy link

xKesvaL commented Oct 16, 2025

Hey! Thanks for the quick PR. Do you think this can fix #5498 too ?

@schiller-manuel schiller-manuel merged commit afaecfc into main Oct 16, 2025
6 checks passed
@schiller-manuel schiller-manuel deleted the index-dynamic-grouped branch October 16, 2025 18:25
Sign up for free to 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.

Index in dynamic grouped layout does not render

3 participants