Skip to content

Commit 043e576

Browse files
Sheraffautofix-ci[bot]nlynzaad
authored
refactor(router-core): simplify matchId (#5807)
* refactor(router-core): simplify matchId * ci: apply automated fixes * update tests * matchId !== routeId * nitpicks * missed in solid router Match --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Nico Lynzaad <[email protected]>
1 parent 1a3bffa commit 043e576

File tree

13 files changed

+40
-110
lines changed

13 files changed

+40
-110
lines changed

‎e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { expect, test } from '@playwright/test'
2-
import { useExperimentalNonNestedRoutes } from './utils/useExperimentalNonNestedRoutes'
32

43
const testCases: Array<{
54
name: string
@@ -185,13 +184,7 @@ test.describe('Non-nested paths', () => {
185184
await expect(pathRouteHeading).not.toBeVisible()
186185
await expect(barHeading).toBeVisible()
187186
const bar2ParamValue = await barParams.innerText()
188-
if (useExperimentalNonNestedRoutes || testPathDesc !== 'named') {
189-
expect(JSON.parse(bar2ParamValue)).toEqual(paramValue2)
190-
} else {
191-
// this is a bug with named path params and non-nested paths
192-
// that is resolved in the new experimental flag
193-
expect(JSON.parse(bar2ParamValue)).toEqual(paramValue)
194-
}
187+
expect(JSON.parse(bar2ParamValue)).toEqual(paramValue2)
195188
})
196189
})
197190
},
@@ -350,17 +343,8 @@ test.describe('Deeply nested non-nested paths', () => {
350343
await expect(bazBarFooRouteHeading).not.toBeVisible()
351344
await expect(bazBarFooQuxHeading).toBeVisible()
352345
await expect(bazBarFooQuxParams).toBeVisible()
353-
354-
if (useExperimentalNonNestedRoutes) {
355-
expect(await bazBarFooQuxParams.innerText()).toBe(
356-
JSON.stringify({ baz: 'baz-bar-qux', foo: 'foo' }),
357-
)
358-
} else {
359-
// this is a bug with named path params and non-nested paths
360-
// that is resolved in the new experimental flag
361-
expect(await bazBarFooQuxParams.innerText()).toBe(
362-
JSON.stringify({ baz: 'baz-bar', foo: 'foo' }),
363-
)
364-
}
346+
expect(await bazBarFooQuxParams.innerText()).toBe(
347+
JSON.stringify({ baz: 'baz-bar-qux', foo: 'foo' }),
348+
)
365349
})
366350
})

‎e2e/react-router/basic-file-based/tests/params.spec.ts‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { expect, test } from '@playwright/test'
2-
import { useExperimentalNonNestedRoutes } from './utils/useExperimentalNonNestedRoutes'
32
import type { Page } from '@playwright/test'
43

54
test.beforeEach(async ({ page }) => {
@@ -100,22 +99,12 @@ test.describe('params operations + non-nested routes', () => {
10099
const foo2ParamsValue = page.getByTestId('foo-params-value')
101100
const foo2ParamsText = await foo2ParamsValue.innerText()
102101
const foo2ParamsObj = JSON.parse(foo2ParamsText)
103-
if (useExperimentalNonNestedRoutes) {
104-
expect(foo2ParamsObj).toEqual({ foo: 'foo2' })
105-
} else {
106-
// this is a bug that is resolved in the new experimental flag
107-
expect(foo2ParamsObj).toEqual({ foo: 'foo' })
108-
}
102+
expect(foo2ParamsObj).toEqual({ foo: 'foo2' })
109103

110104
const params2Value = page.getByTestId('foo-bar-params-value')
111105
const params2Text = await params2Value.innerText()
112106
const params2Obj = JSON.parse(params2Text)
113-
if (useExperimentalNonNestedRoutes) {
114-
expect(params2Obj).toEqual({ foo: 'foo2', bar: 'bar2' })
115-
} else {
116-
// this is a bug that is resolved in the new experimental flag
117-
expect(params2Obj).toEqual({ foo: 'foo', bar: 'bar2' })
118-
}
107+
expect(params2Obj).toEqual({ foo: 'foo2', bar: 'bar2' })
119108
})
120109
})
121110

‎e2e/react-start/basic/tests/navigation.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ test.use({
99
})
1010
test('Navigating to post', async ({ page }) => {
1111
await page.goto('/')
12+
await page.waitForURL('/')
1213

1314
await page.getByRole('link', { name: 'Posts' }).click()
1415
await page.getByRole('link', { name: 'sunt aut facere repe' }).click()

‎e2e/solid-router/basic-file-based/tests/non-nested-paths.spec.ts‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { expect, test } from '@playwright/test'
2-
import { useExperimentalNonNestedRoutes } from './utils/useExperimentalNonNestedRoutes'
32

43
const testCases: Array<{
54
name: string
@@ -185,13 +184,7 @@ test.describe('Non-nested paths', () => {
185184
await expect(pathRouteHeading).not.toBeVisible()
186185
await expect(barHeading).toBeVisible()
187186
const bar2ParamValue = await barParams.innerText()
188-
if (useExperimentalNonNestedRoutes || testPathDesc !== 'named') {
189-
expect(JSON.parse(bar2ParamValue)).toEqual(paramValue2)
190-
} else {
191-
// this is a bug with named path params and non-nested paths
192-
// that is resolved in the new experimental flag
193-
expect(JSON.parse(bar2ParamValue)).toEqual(paramValue)
194-
}
187+
expect(JSON.parse(bar2ParamValue)).toEqual(paramValue2)
195188
})
196189
})
197190
},
@@ -350,17 +343,8 @@ test.describe('Deeply nested non-nested paths', () => {
350343
await expect(bazBarFooRouteHeading).not.toBeVisible()
351344
await expect(bazBarFooQuxHeading).toBeVisible()
352345
await expect(bazBarFooQuxParams).toBeVisible()
353-
354-
if (useExperimentalNonNestedRoutes) {
355-
expect(await bazBarFooQuxParams.innerText()).toBe(
356-
JSON.stringify({ baz: 'baz-bar-qux', foo: 'foo' }),
357-
)
358-
} else {
359-
// this is a bug with named path params and non-nested paths
360-
// that is resolved in the new experimental flag
361-
expect(await bazBarFooQuxParams.innerText()).toBe(
362-
JSON.stringify({ baz: 'baz-bar', foo: 'foo' }),
363-
)
364-
}
346+
expect(await bazBarFooQuxParams.innerText()).toBe(
347+
JSON.stringify({ baz: 'baz-bar-qux', foo: 'foo' }),
348+
)
365349
})
366350
})

‎e2e/solid-router/basic-file-based/tests/params.spec.ts‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { expect, test } from '@playwright/test'
2-
import { useExperimentalNonNestedRoutes } from './utils/useExperimentalNonNestedRoutes'
32
import type { Page } from '@playwright/test'
43

54
test.beforeEach(async ({ page }) => {
@@ -100,22 +99,12 @@ test.describe('params operations + non-nested routes', () => {
10099
const foo2ParamsValue = page.getByTestId('foo-params-value')
101100
const foo2ParamsText = await foo2ParamsValue.innerText()
102101
const foo2ParamsObj = JSON.parse(foo2ParamsText)
103-
if (useExperimentalNonNestedRoutes) {
104-
expect(foo2ParamsObj).toEqual({ foo: 'foo2' })
105-
} else {
106-
// this is a bug that is resolved in the new experimental flag
107-
expect(foo2ParamsObj).toEqual({ foo: 'foo' })
108-
}
102+
expect(foo2ParamsObj).toEqual({ foo: 'foo2' })
109103

110104
const params2Value = page.getByTestId('foo-bar-params-value')
111105
const params2Text = await params2Value.innerText()
112106
const params2Obj = JSON.parse(params2Text)
113-
if (useExperimentalNonNestedRoutes) {
114-
expect(params2Obj).toEqual({ foo: 'foo2', bar: 'bar2' })
115-
} else {
116-
// this is a bug that is resolved in the new experimental flag
117-
expect(params2Obj).toEqual({ foo: 'foo', bar: 'bar2' })
118-
}
107+
expect(params2Obj).toEqual({ foo: 'foo2', bar: 'bar2' })
119108
})
120109
})
121110

‎packages/react-router/src/Match.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ export const Outlet = React.memo(function OutletImpl() {
355355

356356
const nextMatch = <Match matchId={childMatchId} />
357357

358-
if (matchId === rootRouteId) {
358+
if (routeId === rootRouteId) {
359359
return (
360360
<React.Suspense fallback={pendingElement}>{nextMatch}</React.Suspense>
361361
)

‎packages/router-core/src/load-matches.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ const isBeforeLoadSsr = (
213213

214214
// in SPA mode, only SSR the root route
215215
if (inner.router.isShell()) {
216-
existingMatch.ssr = matchId === rootRouteId
216+
existingMatch.ssr = route.id === rootRouteId
217217
return
218218
}
219219

‎packages/router-core/src/path.ts‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,6 @@ function baseParsePathname(pathname: string): ReadonlyArray<Segment> {
377377
interface InterpolatePathOptions {
378378
path?: string
379379
params: Record<string, unknown>
380-
leaveWildcards?: boolean
381380
leaveParams?: boolean
382381
// Map of encoded chars to decoded chars (e.g. '%40' -> '@') that should remain decoded in path params
383382
decodeCharMap?: Map<string, string>
@@ -403,7 +402,6 @@ type InterPolatePathResult = {
403402
export function interpolatePath({
404403
path,
405404
params,
406-
leaveWildcards,
407405
leaveParams,
408406
decodeCharMap,
409407
parseCache,
@@ -446,9 +444,6 @@ export function interpolatePath({
446444
if (!params._splat) {
447445
isMissingParams = true
448446
// For missing splat parameters, just return the prefix and suffix without the wildcard
449-
if (leaveWildcards) {
450-
return `${segmentPrefix}${segment.value}${segmentSuffix}`
451-
}
452447
// If there is a prefix or suffix, return them joined, otherwise omit the segment
453448
if (segmentPrefix || segmentSuffix) {
454449
return `${segmentPrefix}${segmentSuffix}`
@@ -457,9 +452,6 @@ export function interpolatePath({
457452
}
458453

459454
const value = encodeParam('_splat')
460-
if (leaveWildcards) {
461-
return `${segmentPrefix}${segment.value}${value ?? ''}${segmentSuffix}`
462-
}
463455
return `${segmentPrefix}${value}${segmentSuffix}`
464456
}
465457

@@ -487,9 +479,6 @@ export function interpolatePath({
487479

488480
// Check if optional parameter is missing or undefined
489481
if (!(key in params) || params[key] == null) {
490-
if (leaveWildcards) {
491-
return `${segmentPrefix}${key}${segmentSuffix}`
492-
}
493482
// For optional params with prefix/suffix, keep the prefix/suffix but omit the param
494483
if (segmentPrefix || segmentSuffix) {
495484
return `${segmentPrefix}${segmentSuffix}`
@@ -504,9 +493,6 @@ export function interpolatePath({
504493
const value = encodeParam(segment.value)
505494
return `${segmentPrefix}${segment.value}${value ?? ''}${segmentSuffix}`
506495
}
507-
if (leaveWildcards) {
508-
return `${segmentPrefix}${key}${encodeParam(key) ?? ''}${segmentSuffix}`
509-
}
510496
return `${segmentPrefix}${encodeParam(key) ?? ''}${segmentSuffix}`
511497
}
512498

‎packages/router-core/src/router.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,13 +1364,12 @@ export class RouterCore<
13641364
// Existing matches are matches that are already loaded along with
13651365
// pending matches that are still loading
13661366
const matchId =
1367-
interpolatePath({
1368-
path: route.id,
1369-
params: routeParams,
1370-
leaveWildcards: true,
1371-
decodeCharMap: this.pathParamsDecodeCharMap,
1372-
parseCache: this.parsePathnameCache,
1373-
}).interpolatedPath + loaderDepsHash
1367+
// route.id for disambiguation
1368+
route.id +
1369+
// interpolatedPath for param changes
1370+
interpolatedPath +
1371+
// explicit deps
1372+
loaderDepsHash
13741373

13751374
const existingMatch = this.getMatch(matchId)
13761375

@@ -1690,7 +1689,6 @@ export class RouterCore<
16901689
// This preserves the original parameter syntax including optional parameters
16911690
path: nextTo,
16921691
params: nextParams,
1693-
leaveWildcards: false,
16941692
leaveParams: opts.leaveParams,
16951693
decodeCharMap: this.pathParamsDecodeCharMap,
16961694
parseCache: this.parsePathnameCache,

‎packages/router-core/tests/callbacks.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@ describe('callbacks', () => {
4848
await router.navigate({ to: '/foo' })
4949
expect(onEnter).toHaveBeenNthCalledWith(
5050
1,
51-
expect.objectContaining({ id: '/foo' }),
51+
expect.objectContaining({ id: '/foo/foo' }),
5252
)
5353

5454
// Entering bar
5555
await router.navigate({ to: '/bar' })
5656
expect(onEnter).toHaveBeenNthCalledWith(
5757
2,
58-
expect.objectContaining({ id: '/bar' }),
58+
expect.objectContaining({ id: '/bar/bar' }),
5959
)
6060
})
6161
})
@@ -70,14 +70,14 @@ describe('callbacks', () => {
7070
await router.navigate({ to: '/bar' })
7171
expect(onLeave).toHaveBeenNthCalledWith(
7272
1,
73-
expect.objectContaining({ id: '/foo' }),
73+
expect.objectContaining({ id: '/foo/foo' }),
7474
)
7575

7676
// Leaving bar to foo
7777
await router.navigate({ to: '/foo' })
7878
expect(onLeave).toHaveBeenNthCalledWith(
7979
2,
80-
expect.objectContaining({ id: '/bar' }),
80+
expect.objectContaining({ id: '/bar/bar' }),
8181
)
8282
})
8383
})
@@ -92,14 +92,14 @@ describe('callbacks', () => {
9292
await router.navigate({ to: '/foo', search: { foo: 'baz' } })
9393
expect(onStay).toHaveBeenNthCalledWith(
9494
1,
95-
expect.objectContaining({ id: '/foo', search: { foo: 'baz' } }),
95+
expect.objectContaining({ id: '/foo/foo', search: { foo: 'baz' } }),
9696
)
9797

9898
// Staying on foo
9999
await router.navigate({ to: '/foo', search: { foo: 'quux' } })
100100
expect(onStay).toHaveBeenNthCalledWith(
101101
2,
102-
expect.objectContaining({ id: '/foo', search: { foo: 'quux' } }),
102+
expect.objectContaining({ id: '/foo/foo', search: { foo: 'quux' } }),
103103
)
104104
})
105105
})

0 commit comments

Comments
 (0)