diff --git a/packages/react-router/tests/link.test.tsx b/packages/react-router/tests/link.test.tsx index b9c739becd6..468540a3e36 100644 --- a/packages/react-router/tests/link.test.tsx +++ b/packages/react-router/tests/link.test.tsx @@ -5314,6 +5314,60 @@ describe('Link', () => { await runTest({ expectedPreload: false, testIdToHover: 'link-2' }) }) }) + + test('edge-case: competing optional segment links', async () => { + const rootRoute = createRootRoute({ + component: () => { + return ( + <> + To index + + To /foo/bar + + To /bar + + + ) + }, + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>

index

, + }) + const OptRouteComponent = () => { + const params = useParams({ strict: false }) + return 
{JSON.stringify(params)}
+ } + const optRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/{-$foo}/bar', + component: OptRouteComponent, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, optRoute]), + history, + }) + + render() + expect(await screen.findByText('index')).toBeInTheDocument() + + const indexLink = await screen.findByRole('link', { name: 'To index' }) + const fooBarLink = await screen.findByRole('link', { name: 'To /foo/bar' }) + const barLink = await screen.findByRole('link', { name: 'To /bar' }) + + fireEvent.click(fooBarLink) + expect(fooBarLink).toHaveAttribute('aria-current', 'page') + expect(barLink).not.toHaveAttribute('aria-current', 'page') + + fireEvent.click(indexLink) + expect(indexLink).toHaveAttribute('aria-current', 'page') + + fireEvent.click(barLink) + expect(fooBarLink).not.toHaveAttribute('aria-current', 'page') + expect(barLink).toHaveAttribute('aria-current', 'page') + }) }) describe('createLink', () => { diff --git a/packages/react-router/tests/navigate.test.tsx b/packages/react-router/tests/navigate.test.tsx index dd7b252c48f..53dc21d0c17 100644 --- a/packages/react-router/tests/navigate.test.tsx +++ b/packages/react-router/tests/navigate.test.tsx @@ -195,6 +195,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { projectId: 'query' }, }) @@ -213,6 +214,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { projectId: 'query' }, } as any) await router.invalidate() @@ -230,6 +232,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { version: 'v3' }, }) @@ -248,6 +251,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { version: 'v3' }, } as any) await router.invalidate() @@ -265,6 +269,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { framework: 'vue' }, }) @@ -283,6 +288,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { framework: 'vue' }, } as any) await router.invalidate() @@ -778,7 +784,7 @@ describe('router.navigate navigation using optional path parameters - object syn expect(router.state.location.pathname).toBe('/p/router/vue') }) - it('should carry over optional parameters from current route when using empty params', async () => { + it('should carry over optional parameters from current route when params is true', async () => { const { router } = createOptionalParamTestRouter( createMemoryHistory({ initialEntries: ['/posts/tech'] }), ) @@ -798,7 +804,7 @@ describe('router.navigate navigation using optional path parameters - object syn // Navigate back to posts - should carry over 'news' from current params await router.navigate({ to: '/posts/{-$category}', - params: {}, + params: true, }) await router.invalidate() @@ -1039,7 +1045,7 @@ describe('router.navigate navigation using optional path parameters - parameter // Navigate to articles without specifying category await router.navigate({ to: '/articles/{-$category}', - params: {}, + params: true, }) await router.invalidate() @@ -1067,7 +1073,7 @@ describe('router.navigate navigation using optional path parameters - parameter // Navigate back to posts without explicit category removal await router.navigate({ to: '/posts/{-$category}', - params: {}, + params: true, }) await router.invalidate() diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index bd8a2898ec4..0b2152a8eed 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1860,15 +1860,20 @@ export class RouterCore< : sourcePath // Resolve the next params - const nextParams = - dest.params === false || dest.params === null - ? Object.create(null) - : (dest.params ?? true) === true - ? fromParams - : Object.assign( - fromParams, - functionalUpdate(dest.params as any, fromParams), - ) + const inheritParams = + dest.params === true || + typeof dest.params === 'function' || + ((dest.from || !isAbsoluteTo) && + dest.params !== false && + dest.params !== null) + const baseParams = inheritParams ? fromParams : Object.create(null) + const nextParams = inheritParams + ? dest.params === true + ? baseParams + : Object.assign(baseParams, functionalUpdate(dest.params, fromParams)) + : dest.params + ? Object.assign(baseParams, dest.params) + : baseParams const destRoute = this.routesByPath[ trimPathRight(nextTo) as keyof typeof this.routesByPath diff --git a/packages/router-core/tests/build-location.test.ts b/packages/router-core/tests/build-location.test.ts index c8c12ce42e2..8877bad0307 100644 --- a/packages/router-core/tests/build-location.test.ts +++ b/packages/router-core/tests/build-location.test.ts @@ -1036,6 +1036,141 @@ describe('buildLocation - params edge cases', () => { expect(location.pathname).toBe('/users/456') }) + test('omitted params should not inherit current params without from', async () => { + const rootRoute = new BaseRootRoute({}) + const orgRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/orgs/$orgId', + }) + const userRoute = new BaseRoute({ + getParentRoute: () => orgRoute, + path: '/users/$userId', + }) + + const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }), + }) + + await router.load() + + const location = router.buildLocation({ + to: '/orgs/$orgId/users/$userId', + }) + + expect(location.pathname).toBe('/orgs/undefined/users/undefined') + }) + + test('omitted params should not inherit current optional params without from', async () => { + const rootRoute = new BaseRootRoute({}) + const optRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/{-$foo}/bar', + }) + + const routeTree = rootRoute.addChildren([optRoute]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/foo/bar'] }), + }) + + await router.load() + + const location = router.buildLocation({ + to: '/{-$foo}/bar', + }) + + expect(location.pathname).toBe('/bar') + }) + + test('params as object should not merge current params without from', async () => { + const rootRoute = new BaseRootRoute({}) + const orgRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/orgs/$orgId', + }) + const userRoute = new BaseRoute({ + getParentRoute: () => orgRoute, + path: '/users/$userId', + }) + + const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }), + }) + + await router.load() + + const location = router.buildLocation({ + to: '/orgs/$orgId/users/$userId', + params: { userId: '456' }, + }) + + expect(location.pathname).toBe('/orgs/undefined/users/456') + }) + + test('omitted params should inherit params with explicit from', async () => { + const rootRoute = new BaseRootRoute({}) + const orgRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/orgs/$orgId', + }) + const userRoute = new BaseRoute({ + getParentRoute: () => orgRoute, + path: '/users/$userId', + }) + + const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }), + }) + + await router.load() + + const location = router.buildLocation({ + from: '/orgs/$orgId/users/$userId', + to: '/orgs/$orgId/users/$userId', + }) + + expect(location.pathname).toBe('/orgs/abc/users/123') + }) + + test('params as object should merge params with explicit from', async () => { + const rootRoute = new BaseRootRoute({}) + const orgRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/orgs/$orgId', + }) + const userRoute = new BaseRoute({ + getParentRoute: () => orgRoute, + path: '/users/$userId', + }) + + const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }), + }) + + await router.load() + + const location = router.buildLocation({ + from: '/orgs/$orgId/users/$userId', + to: '/orgs/$orgId/users/$userId', + params: { userId: '456' }, + }) + + expect(location.pathname).toBe('/orgs/abc/users/456') + }) + test('params as object should merge with current params', async () => { const rootRoute = new BaseRootRoute({}) const orgRoute = new BaseRoute({ diff --git a/packages/solid-router/tests/link.test.tsx b/packages/solid-router/tests/link.test.tsx index d0fc7ae4697..2d1a628221a 100644 --- a/packages/solid-router/tests/link.test.tsx +++ b/packages/solid-router/tests/link.test.tsx @@ -5286,6 +5286,63 @@ describe('Link', () => { await runTest({ expectedPreload: false, testIdToHover: 'link-2' }) }) }) + + test('edge-case: competing optional segment links', async () => { + const rootRoute = createRootRoute({ + component: () => { + return ( + <> + To index + + To /foo/bar + + To /bar + + + ) + }, + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>

index

, + }) + const optRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/{-$foo}/bar', + component: () => { + const params = useParams({ strict: false }) + return 
{JSON.stringify(params())}
+ }, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, optRoute]), + history, + }) + + render(() => ) + expect(await screen.findByText('index')).toBeInTheDocument() + + const indexLink = await screen.findByRole('link', { name: 'To index' }) + const fooBarLink = await screen.findByRole('link', { name: 'To /foo/bar' }) + const barLink = await screen.findByRole('link', { name: 'To /bar' }) + + await fireEvent.click(fooBarLink) + await waitFor(() => + expect(fooBarLink).toHaveAttribute('aria-current', 'page'), + ) + expect(barLink).not.toHaveAttribute('aria-current', 'page') + + await fireEvent.click(indexLink) + await waitFor(() => + expect(indexLink).toHaveAttribute('aria-current', 'page'), + ) + + await fireEvent.click(barLink) + await waitFor(() => expect(barLink).toHaveAttribute('aria-current', 'page')) + expect(fooBarLink).not.toHaveAttribute('aria-current', 'page') + }) }) describe('createLink', () => { diff --git a/packages/solid-router/tests/navigate.test.tsx b/packages/solid-router/tests/navigate.test.tsx index fdc85abb62e..2f144dcd31f 100644 --- a/packages/solid-router/tests/navigate.test.tsx +++ b/packages/solid-router/tests/navigate.test.tsx @@ -196,6 +196,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { projectId: 'query' }, }) @@ -214,6 +215,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { projectId: 'query' }, } as any) await router.invalidate() @@ -231,6 +233,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { version: 'v3' }, }) @@ -249,6 +252,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { version: 'v3' }, } as any) await router.invalidate() @@ -266,6 +270,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { framework: 'vue' }, }) @@ -284,6 +289,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { framework: 'vue' }, } as any) await router.invalidate() @@ -802,7 +808,7 @@ describe('router.navigate navigation using optional path parameters - object syn expect(router.state.location.pathname).toBe('/p/router/vue') }) - it('should carry over optional parameters from current route when using empty params', async () => { + it('should carry over optional parameters from current route when params is true', async () => { const { router } = createOptionalParamTestRouter( createMemoryHistory({ initialEntries: ['/posts/tech'] }), ) @@ -822,7 +828,7 @@ describe('router.navigate navigation using optional path parameters - object syn // Navigate back to posts - should carry over 'news' from current params await router.navigate({ to: '/posts/{-$category}', - params: {}, + params: true, }) await router.invalidate() @@ -1063,7 +1069,7 @@ describe('router.navigate navigation using optional path parameters - parameter // Navigate to articles without specifying category await router.navigate({ to: '/articles/{-$category}', - params: {}, + params: true, }) await router.invalidate() @@ -1091,7 +1097,7 @@ describe('router.navigate navigation using optional path parameters - parameter // Navigate back to posts without explicit category removal await router.navigate({ to: '/posts/{-$category}', - params: {}, + params: true, }) await router.invalidate() diff --git a/packages/vue-router/tests/link.test.tsx b/packages/vue-router/tests/link.test.tsx index e047fe59933..6b506248d32 100644 --- a/packages/vue-router/tests/link.test.tsx +++ b/packages/vue-router/tests/link.test.tsx @@ -5446,6 +5446,63 @@ describe('Link', () => { await runTest({ expectedPreload: false, testIdToHover: 'link-2' }) }) }) + + test('edge-case: competing optional segment links', async () => { + const rootRoute = createRootRoute({ + component: () => { + return ( + <> + To index + + To /foo/bar + + To /bar + + + ) + }, + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>

index

, + }) + const optRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/{-$foo}/bar', + component: () => { + const params = useParams({ strict: false }) + return 
{JSON.stringify(params.value)}
+ }, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, optRoute]), + history, + }) + + render() + expect(await screen.findByText('index')).toBeInTheDocument() + + const indexLink = await screen.findByRole('link', { name: 'To index' }) + const fooBarLink = await screen.findByRole('link', { name: 'To /foo/bar' }) + const barLink = await screen.findByRole('link', { name: 'To /bar' }) + + await fireEvent.click(fooBarLink) + await waitFor(() => + expect(fooBarLink).toHaveAttribute('aria-current', 'page'), + ) + expect(barLink).not.toHaveAttribute('aria-current', 'page') + + await fireEvent.click(indexLink) + await waitFor(() => + expect(indexLink).toHaveAttribute('aria-current', 'page'), + ) + + await fireEvent.click(barLink) + await waitFor(() => expect(barLink).toHaveAttribute('aria-current', 'page')) + expect(fooBarLink).not.toHaveAttribute('aria-current', 'page') + }) }) describe('createLink', () => { diff --git a/packages/vue-router/tests/navigate.test.tsx b/packages/vue-router/tests/navigate.test.tsx index b66e604dc55..89099e511ae 100644 --- a/packages/vue-router/tests/navigate.test.tsx +++ b/packages/vue-router/tests/navigate.test.tsx @@ -196,6 +196,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { projectId: 'query' }, }) @@ -214,6 +215,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { projectId: 'query' }, } as any) await router.invalidate() @@ -231,6 +233,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { version: 'v3' }, }) @@ -249,6 +252,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { version: 'v3' }, } as any) await router.invalidate() @@ -266,6 +270,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', to: '/p/$projectId/$version/$framework', params: { framework: 'vue' }, }) @@ -284,6 +289,7 @@ describe('router.navigate navigation using multiple path params - object syntax expect(router.state.location.pathname).toBe('/p/router/v1/react') await router.navigate({ + from: '/p/$projectId/$version/$framework', params: { framework: 'vue' }, } as any) await router.invalidate() @@ -802,7 +808,7 @@ describe('router.navigate navigation using optional path parameters - object syn expect(router.state.location.pathname).toBe('/p/router/vue') }) - it('should carry over optional parameters from current route when using empty params', async () => { + it('should carry over optional parameters from current route when params is true', async () => { const { router } = createOptionalParamTestRouter( createMemoryHistory({ initialEntries: ['/posts/tech'] }), ) @@ -822,7 +828,7 @@ describe('router.navigate navigation using optional path parameters - object syn // Navigate back to posts - should carry over 'news' from current params await router.navigate({ to: '/posts/{-$category}', - params: {}, + params: true, }) await router.invalidate() @@ -1063,7 +1069,7 @@ describe('router.navigate navigation using optional path parameters - parameter // Navigate to articles without specifying category await router.navigate({ to: '/articles/{-$category}', - params: {}, + params: true, }) await router.invalidate() @@ -1091,7 +1097,7 @@ describe('router.navigate navigation using optional path parameters - parameter // Navigate back to posts without explicit category removal await router.navigate({ to: '/posts/{-$category}', - params: {}, + params: true, }) await router.invalidate()