diff --git a/.changeset/still-links-rest.md b/.changeset/still-links-rest.md index 770daa9ba9..830506b9c4 100644 --- a/.changeset/still-links-rest.md +++ b/.changeset/still-links-rest.md @@ -3,4 +3,4 @@ '@tanstack/react-router': patch --- -Reuse built locations for Links whose destination does not depend on the current location. `buildLocation` keeps the result per options object when the build never read the current location, and the React `Link` passes one stable options object per instance, so navigations resolve unchanged Links with a lookup instead of a full build. The per-route pathname interpolation cache this replaces is removed. Link `params`, `search` and `activeOptions` are compared by value on render; an object mutated in place is picked up on the next render rather than by a navigation alone. +Reuse built locations for Links whose destination does not depend on the current location. `buildLocation` keeps the result per options object when the build never read the current location, and the React `Link` passes one stable options object per instance, so navigations resolve unchanged Links with a lookup instead of a full build. The per-route pathname interpolation cache this replaces is removed. Link `params`, `search` and `activeOptions` are compared by value on render, so inline object literals with unchanged contents keep reusing the Link's location. Pass a new object to change a destination; like any other React prop, an object mutated in place is not re-read. diff --git a/packages/react-router/src/link.tsx b/packages/react-router/src/link.tsx index 2d63f385b2..7420ba71ea 100644 --- a/packages/react-router/src/link.tsx +++ b/packages/react-router/src/link.tsx @@ -40,20 +40,21 @@ type LinkState = [href: string | undefined, isActive?: boolean] // otherwise change `_options` identity on every parent render, rebuild the // store selector, and discard its memoized selection. // -// The kept value is a shallow copy, never the caller's object: the router -// reuses a built location for as long as it sees the same options object, so -// a params object mutated in place, or one backed by accessors, has to yield -// a new reference here on the render that observes the change. +// The router reuses a built location for as long as it sees the same options +// object, so the reference returned here is its invalidation signal: pass a +// new object to change a destination. Like every other React prop, an object +// mutated in place is not re-read. `deepEqual` short-circuits on reference +// equality, so an unchanged reference costs nothing. // // `ignoreUndefined: false` is required: an explicit `undefined` clears an // inherited param or search key, so `{}` and `{ category: undefined }` build // different locations and must not be treated as equal here. function useValueStable(value: T): T { - const ref = React.useRef(undefined) + const ref = React.useRef(value) if (!deepEqual(ref.current, value, { ignoreUndefined: false })) { - ref.current = value && typeof value === 'object' ? { ...value } : value + ref.current = value } - return ref.current as T + return ref.current } function compareLinkState(a: LinkState, b: LinkState) { diff --git a/packages/react-router/tests/link-destination.test.tsx b/packages/react-router/tests/link-destination.test.tsx index 6447e5aa06..f33fe65803 100644 --- a/packages/react-router/tests/link-destination.test.tsx +++ b/packages/react-router/tests/link-destination.test.tsx @@ -24,25 +24,21 @@ describe('Link destination updates', () => { stringify?: (params: Record) => { id: string }, ) { const rootRoute = createRootRoute({ - component: function Root() { - const [, rerender] = React.useState(0) - return ( - <> - - - Target - - - - ) - }, + component: () => ( + <> + + Target + + + + ), }) const itemsRoute = createRoute({ getParentRoute: () => rootRoute, @@ -167,40 +163,73 @@ describe('Link destination updates', () => { expect(link).toHaveAttribute('href', '/target/fixed#details') }) - test('reads accessor-backed params on the next render', async () => { - let id = 'one' - const { router } = setupFixedLink({ - get id() { - return id + test('reuses the location for equal inline literals and rebuilds on nested changes', async () => { + const rootRoute = createRootRoute({ + component: function Root() { + const [page, setPage] = React.useState(1) + const [, rerender] = React.useState(0) + return ( + <> + + + + Items + + + + ) }, }) + const itemsRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/items/$source', + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([itemsRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + const buildLocation = vi.spyOn(router, 'buildLocation') render() - const link = await screen.findByTestId('fixed-link') - expect(link).toHaveAttribute('href', '/target/one#details') + const link = await screen.findByTestId('nested-link') + expect(link).toHaveAttribute( + 'href', + '/items/one?filters=%7B%22page%22%3A1%7D&tags=%5B%22a%22%5D', + ) - id = 'two' + // Fresh literals with equal contents must not produce a new options + // object, otherwise the router could never reuse the built location. + const builds = buildLocation.mock.calls.length + expect(builds).toBeGreaterThan(0) fireEvent.click(screen.getByRole('button', { name: 'Rerender' })) - expect(link).toHaveAttribute('href', '/target/two#details') - }) - - test('updates when an existing params object changes before a render', async () => { - const params = { id: 'one' } - const { router } = setupFixedLink(params) - render() - - const link = await screen.findByTestId('fixed-link') - expect(link).toHaveAttribute('href', '/target/one#details') - - params.id = 'two' fireEvent.click(screen.getByRole('button', { name: 'Rerender' })) - expect(link).toHaveAttribute('href', '/target/two#details') + expect(buildLocation).toHaveBeenCalledTimes(builds) + expect(link).toHaveAttribute( + 'href', + '/items/one?filters=%7B%22page%22%3A1%7D&tags=%5B%22a%22%5D', + ) + + // A nested value that changes through React state is a new object, so + // the cached location for the previous contents must not be served. + fireEvent.click(screen.getByRole('button', { name: 'Next page' })) + expect(link).toHaveAttribute( + 'href', + '/items/one?filters=%7B%22page%22%3A2%7D&tags=%5B%22a%22%5D', + ) - // The same object keeps its location across navigations until it changes again. await act(() => router.navigate({ to: '/items/$source', params: { source: 'two' } }), ) - expect(link).toHaveAttribute('href', '/target/two#details') + expect(link).toHaveAttribute( + 'href', + '/items/one?filters=%7B%22page%22%3A2%7D&tags=%5B%22a%22%5D', + ) + buildLocation.mockRestore() }) test('updates fixed params and hash when Link props change', async () => {