From f768adfa7cfd33f97f79301c3935f3f77a2c8a91 Mon Sep 17 00:00:00 2001 From: Nico Lynzaad Date: Thu, 18 Sep 2025 03:57:25 +0200 Subject: [PATCH 1/3] parse non-nested path params correctly --- .../src/routes/params-ps/non-nested/route.tsx | 8 +++++++ .../basic-file-based/tests/params.spec.ts | 22 +++++++++++++++++++ .../src/routes/params-ps/non-nested/route.tsx | 10 ++++++++- .../basic-file-based/tests/params.spec.ts | 21 ++++++++++++++++++ packages/router-core/src/path.ts | 14 +++++++++++- packages/router-core/tests/path.test.ts | 6 +++++ 6 files changed, 79 insertions(+), 2 deletions(-) diff --git a/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx b/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx index 3760bde7088..a8fde97b61c 100644 --- a/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx +++ b/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx @@ -18,6 +18,14 @@ function RouteComponent() { > /params-ps/non-nested/$foo/$bar + + /params-ps/non-nested/$foo/$bar + diff --git a/e2e/react-router/basic-file-based/tests/params.spec.ts b/e2e/react-router/basic-file-based/tests/params.spec.ts index d4d6d2e602b..333ca7333e4 100644 --- a/e2e/react-router/basic-file-based/tests/params.spec.ts +++ b/e2e/react-router/basic-file-based/tests/params.spec.ts @@ -69,6 +69,7 @@ test.describe('params operations + non-nested routes', () => { 'href', '/params-ps/non-nested/foo/bar', ) + await fooBarLink.click() await page.waitForLoadState('networkidle') const pagePathname = new URL(page.url()).pathname @@ -83,6 +84,27 @@ test.describe('params operations + non-nested routes', () => { const paramsText = await paramsValue.innerText() const paramsObj = JSON.parse(paramsText) expect(paramsObj).toEqual({ foo: 'foo', bar: 'bar' }) + + const foo2Bar2Link = page.getByTestId('l-to-non-nested-foo2-bar2') + + await expect(foo2Bar2Link).toHaveAttribute( + 'href', + '/params-ps/non-nested/foo2/bar2', + ) + await foo2Bar2Link.click() + const pagePathname2 = new URL(page.url()).pathname + await page.waitForLoadState('networkidle') + expect(pagePathname2).toBe('/params-ps/non-nested/foo2/bar2') + + const foo2ParamsValue = page.getByTestId('foo-params-value') + const foo2ParamsText = await foo2ParamsValue.innerText() + const foo2ParamsObj = JSON.parse(foo2ParamsText) + expect(foo2ParamsObj).toEqual({ foo: 'foo2' }) + + const params2Value = page.getByTestId('foo-bar-params-value') + const params2Text = await params2Value.innerText() + const params2Obj = JSON.parse(params2Text) + expect(params2Obj).toEqual({ foo: 'foo2', bar: 'bar2' }) }) }) diff --git a/e2e/solid-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx b/e2e/solid-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx index 496542c6677..6de7393593a 100644 --- a/e2e/solid-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx +++ b/e2e/solid-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx @@ -16,7 +16,15 @@ function RouteComponent() { to="./$foo/$bar" params={{ foo: 'foo', bar: 'bar' }} > - /params-ps/non-nested/$foo/$bar + /params-ps/non-nested/foo/bar + + + /params-ps/non-nested/foo2/bar2 diff --git a/e2e/solid-router/basic-file-based/tests/params.spec.ts b/e2e/solid-router/basic-file-based/tests/params.spec.ts index 3a63c934175..6a1267f1153 100644 --- a/e2e/solid-router/basic-file-based/tests/params.spec.ts +++ b/e2e/solid-router/basic-file-based/tests/params.spec.ts @@ -158,5 +158,26 @@ test.describe('params operations + non-nested routes', () => { const paramsText = await paramsValue.innerText() const paramsObj = JSON.parse(paramsText) expect(paramsObj).toEqual({ foo: 'foo', bar: 'bar' }) + + const foo2Bar2Link = page.getByTestId('l-to-non-nested-foo2-bar2') + + await expect(foo2Bar2Link).toHaveAttribute( + 'href', + '/params-ps/non-nested/foo2/bar2', + ) + await foo2Bar2Link.click() + const pagePathname2 = new URL(page.url()).pathname + await page.waitForLoadState('networkidle') + expect(pagePathname2).toBe('/params-ps/non-nested/foo2/bar2') + + const foo2ParamsValue = page.getByTestId('foo-params-value') + const foo2ParamsText = await foo2ParamsValue.innerText() + const foo2ParamsObj = JSON.parse(foo2ParamsText) + expect(foo2ParamsObj).toEqual({ foo: 'foo2' }) + + const params2Value = page.getByTestId('foo-bar-params-value') + const params2Text = await params2Value.innerText() + const params2Obj = JSON.parse(params2Text) + expect(params2Obj).toEqual({ foo: 'foo2', bar: 'bar2' }) }) }) diff --git a/packages/router-core/src/path.ts b/packages/router-core/src/path.ts index 7402001cdf4..a302390c685 100644 --- a/packages/router-core/src/path.ts +++ b/packages/router-core/src/path.ts @@ -218,7 +218,8 @@ export const parsePathname = ( return parsed } -const PARAM_RE = /^\$.{1,}$/ // $paramName +const PARAM_RE = /^\$.{1,}(? { } } + // Check for non-nested bare parameter format: $paramName_ (without curly braces) + if (PARAM_NON_NESTED_RE.test(part)) { + const paramName = part.slice(1, -1) + return { + type: SEGMENT_TYPE_PARAM, + value: '$' + paramName, + prefixSegment: undefined, + suffixSegment: undefined, + } + } + // Check for bare wildcard: $ (without curly braces) if (WILDCARD_RE.test(part)) { return { diff --git a/packages/router-core/tests/path.test.ts b/packages/router-core/tests/path.test.ts index 2a241bcb77d..6e416542f88 100644 --- a/packages/router-core/tests/path.test.ts +++ b/packages/router-core/tests/path.test.ts @@ -432,6 +432,12 @@ describe('interpolatePath', () => { params: { _splat: 'sean/cassiere' }, result: '/users/sean/cassiere', }, + { + name: 'should interpolate the non-nested path', + path: '/users/$id_', + params: { id: '123' }, + result: '/users/123', + }, ])('$name', ({ path, params, decodeCharMap, result }) => { expect( interpolatePath({ From a3c98d3238288f73517a4647857d87adb29a443c Mon Sep 17 00:00:00 2001 From: Nico Lynzaad Date: Thu, 18 Sep 2025 09:38:07 +0200 Subject: [PATCH 2/3] code rabbit suggestions --- .../src/routes/params-ps/non-nested/route.tsx | 4 ++-- .../basic-file-based/tests/params.spec.ts | 4 ++-- .../basic-file-based/tests/params.spec.ts | 5 +++-- packages/router-core/src/path.ts | 14 +++++++------- packages/router-core/tests/path.test.ts | 17 +++++++++++++++++ 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx b/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx index a8fde97b61c..93fd9362182 100644 --- a/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx +++ b/e2e/react-router/basic-file-based/src/routes/params-ps/non-nested/route.tsx @@ -16,7 +16,7 @@ function RouteComponent() { to="./$foo/$bar" params={{ foo: 'foo', bar: 'bar' }} > - /params-ps/non-nested/$foo/$bar + /params-ps/non-nested/foo/bar - /params-ps/non-nested/$foo/$bar + /params-ps/non-nested/foo2/bar2 diff --git a/e2e/react-router/basic-file-based/tests/params.spec.ts b/e2e/react-router/basic-file-based/tests/params.spec.ts index 333ca7333e4..76f32a935a8 100644 --- a/e2e/react-router/basic-file-based/tests/params.spec.ts +++ b/e2e/react-router/basic-file-based/tests/params.spec.ts @@ -71,7 +71,7 @@ test.describe('params operations + non-nested routes', () => { ) await fooBarLink.click() - await page.waitForLoadState('networkidle') + await page.waitForURL('/params-ps/non-nested/foo/bar') const pagePathname = new URL(page.url()).pathname expect(pagePathname).toBe('/params-ps/non-nested/foo/bar') @@ -92,8 +92,8 @@ test.describe('params operations + non-nested routes', () => { '/params-ps/non-nested/foo2/bar2', ) await foo2Bar2Link.click() + await page.waitForURL('/params-ps/non-nested/foo2/bar2') const pagePathname2 = new URL(page.url()).pathname - await page.waitForLoadState('networkidle') expect(pagePathname2).toBe('/params-ps/non-nested/foo2/bar2') const foo2ParamsValue = page.getByTestId('foo-params-value') diff --git a/e2e/solid-router/basic-file-based/tests/params.spec.ts b/e2e/solid-router/basic-file-based/tests/params.spec.ts index 6a1267f1153..e85cd0c6b01 100644 --- a/e2e/solid-router/basic-file-based/tests/params.spec.ts +++ b/e2e/solid-router/basic-file-based/tests/params.spec.ts @@ -145,7 +145,8 @@ test.describe('params operations + non-nested routes', () => { '/params-ps/non-nested/foo/bar', ) await fooBarLink.click() - await page.waitForLoadState('networkidle') + await page.waitForURL('/params-ps/non-nested/foo/bar') + const pagePathname = new URL(page.url()).pathname expect(pagePathname).toBe('/params-ps/non-nested/foo/bar') @@ -166,8 +167,8 @@ test.describe('params operations + non-nested routes', () => { '/params-ps/non-nested/foo2/bar2', ) await foo2Bar2Link.click() + await page.waitForURL('/params-ps/non-nested/foo2/bar2') const pagePathname2 = new URL(page.url()).pathname - await page.waitForLoadState('networkidle') expect(pagePathname2).toBe('/params-ps/non-nested/foo2/bar2') const foo2ParamsValue = page.getByTestId('foo-params-value') diff --git a/packages/router-core/src/path.ts b/packages/router-core/src/path.ts index a302390c685..c368a048a9d 100644 --- a/packages/router-core/src/path.ts +++ b/packages/router-core/src/path.ts @@ -218,7 +218,7 @@ export const parsePathname = ( return parsed } -const PARAM_RE = /^\$.{1,}(? { } } - // Check for bare parameter format: $paramName (without curly braces) - if (PARAM_RE.test(part)) { - const paramName = part.substring(1) + // Check for non-nested bare parameter format: $paramName_ (without curly braces) + if (PARAM_NON_NESTED_RE.test(part)) { + const paramName = part.slice(1, -1) return { type: SEGMENT_TYPE_PARAM, value: '$' + paramName, @@ -320,9 +320,9 @@ function baseParsePathname(pathname: string): ReadonlyArray { } } - // Check for non-nested bare parameter format: $paramName_ (without curly braces) - if (PARAM_NON_NESTED_RE.test(part)) { - const paramName = part.slice(1, -1) + // Check for bare parameter format: $paramName (without curly braces) + if (PARAM_RE.test(part)) { + const paramName = part.substring(1) return { type: SEGMENT_TYPE_PARAM, value: '$' + paramName, diff --git a/packages/router-core/tests/path.test.ts b/packages/router-core/tests/path.test.ts index 6e416542f88..d3580cc9a9a 100644 --- a/packages/router-core/tests/path.test.ts +++ b/packages/router-core/tests/path.test.ts @@ -660,6 +660,14 @@ describe('matchPathname', () => { }, expectedMatchedParams: { id: '123' }, }, + { + name: 'should match and return the non-nested named path params', + input: '/users/123', + matchingOptions: { + to: '/users/$id_', + }, + expectedMatchedParams: { id: '123' }, + }, { name: 'should match and return the the splat param', input: '/users/123', @@ -899,6 +907,15 @@ describe('parsePathname', () => { { type: SEGMENT_TYPE_PARAM, value: '$bar' }, ], }, + { + name: 'should handle non-nested named params', + to: '/foo/$bar_', + expected: [ + { type: SEGMENT_TYPE_PATHNAME, value: '/' }, + { type: SEGMENT_TYPE_PATHNAME, value: 'foo' }, + { type: SEGMENT_TYPE_PARAM, value: '$bar' }, + ], + }, { name: 'should handle named params at the root', to: '/$bar', From ebb3ffbd045625894abe72dac4e48e4fcf68da6b Mon Sep 17 00:00:00 2001 From: Nico Lynzaad Date: Thu, 18 Sep 2025 20:09:11 +0200 Subject: [PATCH 3/3] strip trailing "_" for params parsing --- packages/router-core/src/path.ts | 34 ++++++++++++-------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/packages/router-core/src/path.ts b/packages/router-core/src/path.ts index c368a048a9d..b7d312c6b37 100644 --- a/packages/router-core/src/path.ts +++ b/packages/router-core/src/path.ts @@ -218,8 +218,7 @@ export const parsePathname = ( return parsed } -const PARAM_RE = /^\$[^_](.*[^_])?$/ // $paramName -const PARAM_NON_NESTED_RE = /^\$.{1,}_$/ // $paramName_ +const PARAM_RE = /^\$.{1,}$/ // $paramName const PARAM_W_CURLY_BRACES_RE = /^(.*?)\{(\$[a-zA-Z_$][a-zA-Z0-9_$]*)\}(.*)$/ // prefix{$paramName}suffix const OPTIONAL_PARAM_W_CURLY_BRACES_RE = /^(.*?)\{-(\$[a-zA-Z_$][a-zA-Z0-9_$]*)\}(.*)$/ // prefix{-$paramName}suffix @@ -266,8 +265,11 @@ function baseParsePathname(pathname: string): ReadonlyArray { segments.push( ...split.map((part): Segment => { + // strip tailing underscore for non-nested paths + const partToMatch = part.slice(-1) === '_' ? part.slice(0, -1) : part + // Check for wildcard with curly braces: prefix{$}suffix - const wildcardBracesMatch = part.match(WILDCARD_W_CURLY_BRACES_RE) + const wildcardBracesMatch = partToMatch.match(WILDCARD_W_CURLY_BRACES_RE) if (wildcardBracesMatch) { const prefix = wildcardBracesMatch[1] const suffix = wildcardBracesMatch[2] @@ -280,7 +282,7 @@ function baseParsePathname(pathname: string): ReadonlyArray { } // Check for optional parameter format: prefix{-$paramName}suffix - const optionalParamBracesMatch = part.match( + const optionalParamBracesMatch = partToMatch.match( OPTIONAL_PARAM_W_CURLY_BRACES_RE, ) if (optionalParamBracesMatch) { @@ -296,7 +298,7 @@ function baseParsePathname(pathname: string): ReadonlyArray { } // Check for the new parameter format: prefix{$paramName}suffix - const paramBracesMatch = part.match(PARAM_W_CURLY_BRACES_RE) + const paramBracesMatch = partToMatch.match(PARAM_W_CURLY_BRACES_RE) if (paramBracesMatch) { const prefix = paramBracesMatch[1] const paramName = paramBracesMatch[2] @@ -309,20 +311,10 @@ function baseParsePathname(pathname: string): ReadonlyArray { } } - // Check for non-nested bare parameter format: $paramName_ (without curly braces) - if (PARAM_NON_NESTED_RE.test(part)) { - const paramName = part.slice(1, -1) - return { - type: SEGMENT_TYPE_PARAM, - value: '$' + paramName, - prefixSegment: undefined, - suffixSegment: undefined, - } - } - // Check for bare parameter format: $paramName (without curly braces) - if (PARAM_RE.test(part)) { - const paramName = part.substring(1) + if (PARAM_RE.test(partToMatch)) { + const paramName = partToMatch.substring(1) + return { type: SEGMENT_TYPE_PARAM, value: '$' + paramName, @@ -332,7 +324,7 @@ function baseParsePathname(pathname: string): ReadonlyArray { } // Check for bare wildcard: $ (without curly braces) - if (WILDCARD_RE.test(part)) { + if (WILDCARD_RE.test(partToMatch)) { return { type: SEGMENT_TYPE_WILDCARD, value: '$', @@ -344,8 +336,8 @@ function baseParsePathname(pathname: string): ReadonlyArray { // Handle regular pathname segment return { type: SEGMENT_TYPE_PATHNAME, - value: part.includes('%25') - ? part + value: partToMatch.includes('%25') + ? partToMatch .split('%25') .map((segment) => decodeURI(segment)) .join('%25')