fix(router-core): Non-nested paths #5169
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds non-nested route examples to React and Solid E2E apps (/non-nested, /non-nested/baz, dynamic $bazid, and edit variants), updates generated route trees and tests, adjusts router-core path parsing for non-nested segments, and expands router-core tests for matching and path resolution in non-nested scenarios. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant App as App (React / Solid)
participant Router as Router
participant Path as Path Parser
participant RT as Generated Route Tree
U->>App: navigate to /non-nested...
App->>Router: ask for route match
Router->>Path: baseParsePathname(url)
Note over Path: normalize trailing '_' for non-nested segments\nthen decode partToMatch (not original part)
Path-->>Router: segments & params
Router->>RT: lookup nodes (/non-nested, /baz, /$bazid, /edit)
RT-->>Router: matched route node(s)
Router-->>App: render RouteComponent(s) (links, headings, Outlet)
App-->>U: display UI and params
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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). (1)
Comment |
|
View your CI Pipeline Execution ↗ for commit 0fa77f0
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
e2e/solid-router/basic-file-based/tests/app.spec.ts (1)
300-333: Avoid unnecessary awaits on Locators and prefer hidden over viewport checks
getByTestIdreturns a Locator;awaitis unnecessary. For the negative checks,toBeHiddenis less flaky thannot.toBeInViewport().Apply this diff:
- const bazIdLink = await page.getByTestId('l-to-non-nested-bazid') - const bazIdEditLink = await page.getByTestId('l-to-non-nested-bazid-edit') + const bazIdLink = page.getByTestId('l-to-non-nested-bazid') + const bazIdEditLink = page.getByTestId('l-to-non-nested-bazid-edit') @@ - await expect(bazHeading).not.toBeInViewport() - await expect(bazIdHeading).not.toBeInViewport() + await expect(bazHeading).toBeHidden() + await expect(bazIdHeading).toBeHidden()e2e/solid-router/basic-file-based/src/routes/non-nested/route.tsx (1)
14-31: Consider using consistent structure for links.The two links render within the same
<li>element without separation. Consider placing each link in its own list item or adding appropriate styling/separation between them.Consider this improved structure:
<ul class="grid mb-2"> <li> <Link from={Route.fullPath} data-testid="l-to-non-nested-bazid" to="./baz/$bazid" params={{ bazid: '123' }} > /non-nested/baz/123 </Link> + </li> + <li> <Link from={Route.fullPath} data-testid="l-to-non-nested-bazid-edit" to="./baz/$bazid/edit" params={{ bazid: '456' }} > /non-nested/baz/456/edit </Link> </li> </ul>packages/router-core/tests/path.test.ts (1)
1542-1562: Consider adding more edge cases for missing splat params.While the test cases cover basic scenarios with prefix/suffix, consider adding edge cases like empty paths or paths with multiple segments after the splat.
Consider adding these test cases:
{ name: 'nested splat route without _splat', path: '/hello/world/$_', params: {}, expectedResult: '/hello/world', }, { name: 'splat route with multiple prefixes', path: '/hello/prefix1{$}prefix2_', params: {}, expectedResult: '/hello/prefix1prefix2', },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
e2e/react-router/basic-file-based/src/routeTree.gen.ts(39 hunks)e2e/react-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/non-nested/baz.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/non-nested/route.tsx(1 hunks)e2e/react-router/basic-file-based/tests/app.spec.ts(1 hunks)e2e/solid-router/basic-file-based/src/routeTree.gen.ts(39 hunks)e2e/solid-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/non-nested/baz.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/non-nested/route.tsx(1 hunks)e2e/solid-router/basic-file-based/tests/app.spec.ts(1 hunks)e2e/solid-router/basic-file-based/tests/redirect.spec.ts(1 hunks)packages/router-core/src/path.ts(1 hunks)packages/router-core/tests/match-by-path.test.ts(1 hunks)packages/router-core/tests/path.test.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
e2e/react-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (3)
e2e/react-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/route.tsx (1)
Route(3-5)
e2e/react-router/basic-file-based/src/routes/non-nested/baz.tsx (3)
e2e/react-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/route.tsx (1)
Route(3-5)
e2e/solid-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (3)
e2e/solid-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/route.tsx (1)
Route(3-5)
e2e/react-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (3)
e2e/react-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/route.tsx (1)
Route(3-5)
e2e/react-router/basic-file-based/src/routes/non-nested/route.tsx (3)
e2e/react-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
Route(3-5)
e2e/solid-router/basic-file-based/src/routes/non-nested/route.tsx (3)
e2e/solid-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
Route(3-5)
e2e/solid-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (3)
e2e/solid-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/route.tsx (1)
Route(3-5)
e2e/solid-router/basic-file-based/src/routes/non-nested/baz.tsx (3)
e2e/solid-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/non-nested/route.tsx (1)
Route(3-5)
packages/router-core/tests/path.test.ts (1)
packages/router-core/src/path.ts (8)
resolvePath(153-206)trimPathLeft(39-41)interpolatePath(375-489)matchPathname(501-520)SEGMENT_TYPE_PATHNAME(6-6)SEGMENT_TYPE_PARAM(7-7)SEGMENT_TYPE_WILDCARD(8-8)parsePathname(209-219)
⏰ 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 (18)
packages/router-core/src/path.ts (1)
336-345: Correct: decode normalized segment to honor non-nested markerDecoding
partToMatch(post underscore-strip) fixes mismatches for segments likebaz_. Please add a couple of regression cases:
'/non-nested/baz_/$id'vs actual url/non-nested/baz/123- segment containing
%25plus trailing_, e.g.'abc%25def_'→'abc%25def'e2e/react-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
7-23: LGTM: dynamic params surfaced with stable test idsComponent is minimal and test-friendly. No issues.
e2e/react-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
7-21: LGTM: edit route wiring matches testsRoute path and test ids align with the new e2e assertions.
e2e/react-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
7-14: LGTM: parent outlet for non-nested branchParent heading + Outlet behaves as expected for
/non-nested/bazand not for the edit branch.e2e/solid-router/basic-file-based/src/routes/non-nested/baz.tsx (1)
7-14: LGTM: Solid parent with Outlet mirrors React variantParity maintained with React route; good for cross-framework tests.
e2e/solid-router/basic-file-based/tests/redirect.spec.ts (1)
58-59: Good flake fix: wait for network idle after raceThe extra
waitForLoadState('networkidle')stabilizes preload detection before assertions and navigation.e2e/react-router/basic-file-based/src/routes/non-nested/route.tsx (1)
13-35: LGTM: links generated from Route.fullPath; paths align with non-nested behaviorRelative
totargets are clean (no_in URL) and match the updated parsing.packages/router-core/tests/match-by-path.test.ts (1)
201-388: LGTM!The new test suite for non-nested paths comprehensively covers regular path matching, case insensitive matching, and fuzzy matching scenarios, extending the test coverage systematically with wildcard, optional parameter, and static path variations.
e2e/solid-router/basic-file-based/src/routes/non-nested/baz_.$bazid.edit.tsx (1)
1-22: LGTM!The implementation correctly defines the non-nested edit route for Solid Router with appropriate parameter extraction and rendering. The test IDs align well with the test assertions in the app.spec.ts file.
e2e/solid-router/basic-file-based/src/routes/non-nested/baz.$bazid.tsx (1)
1-23: LGTM!The route component follows the expected pattern for dynamic parameter routes in Solid Router, correctly extracting and rendering the
bazidparameter with appropriate test IDs.e2e/react-router/basic-file-based/src/routeTree.gen.ts (3)
24-77: LGTM!The import statements for the new non-nested routes are correctly added and follow the existing naming conventions.
141-201: LGTM!The route definitions for non-nested paths are correctly configured with appropriate parent-child relationships, IDs, and paths.
1424-1448: LGTM!The route hierarchy for NonNestedBazRoute and its children is correctly established, following the expected pattern for nested routes.
e2e/react-router/basic-file-based/tests/app.spec.ts (1)
312-345: LGTM!The end-to-end test thoroughly validates the non-nested path behavior, checking navigation, visibility of components, and correct parameter rendering at each route level.
packages/router-core/tests/path.test.ts (1)
1129-2110: Comprehensive test coverage for non-nested paths!The test suite provides excellent coverage for non-nested paths across all major path operation functions (resolvePath, interpolatePath, matchPathname, parsePathname). The tests thoroughly validate trailing underscore stripping behavior, parameter extraction, and path resolution with various configurations.
e2e/solid-router/basic-file-based/src/routeTree.gen.ts (3)
23-68: LGTM!The import statements for the new non-nested routes correctly follow the existing patterns and naming conventions.
126-180: LGTM!The route definitions are correctly configured with appropriate IDs, paths, and parent relationships matching the expected non-nested structure.
1238-1262: LGTM!The route hierarchy correctly establishes the parent-child relationships for the non-nested routes, matching the structure in the React Router implementation.
nlynzaad
added a commit
that referenced
this pull request
Sep 18, 2025
nlynzaad
added a commit
that referenced
this pull request
Sep 18, 2025
This PR merges the latest updates to main from #5169 and #5165 into Alpha and brings with it an update to how non-nested paths are parsed. It also adds additional unit testing for paths and matchByPath as well as additional e2e tests for react and solid --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
nlynzaad
added a commit
that referenced
this pull request
Sep 22, 2025
…n route segments (#5182) as part of #5169 we made changes to strip underscores from the paths to ensure it is dealt with correctly during parsing. inadvertently this stripped it from not only the route segments but also the base segments. This PR resolves that issue by making `baseParsePathName` aware which type of segments it is parsing. a new optional boolean paramater, ```basePathValues```, is introduced in `baseParsePathName` and `parsePathname` to allow this distinction between routeSegments and/or basesegments to be passed on to `baseParsePathName`. 2 new functions are introduced `parseBasePathSegments` and `parseRoutePathSegments`, which is just wrapper functions around `parsePathname`, but ensures clearer understanding of what set of parsing instructions is applied and `basePathValues `is set correctly based on the requirement. updated the unit tests to take into account the need not to strip the base values and also enhanced the e2e tests as well to ensure more through testing with different usages. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added non-nested route demos (named, prefix, suffix, path) with index/foo/bar pages and updated navigation in React and Solid examples. - Bug Fixes - Improved path parsing/matching to handle non-nested segments, prefixes/suffixes and trailing underscores; refined route sorting/resolution. - Tests - Added comprehensive E2E coverage for non-nested paths; removed obsolete baz-based tests. - Expanded unit tests for path interpolation and matching (underscore, prefix/suffix cases). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Sep 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR continues on from #5165 and primarily adds additional tests to path resolution and matchByPath for non-nested paths in additiona to e2e test for both react and solid router to verify non-nesting functionality.
One additional issue was picked up for handling of normal path segments.
Summary by CodeRabbit