diff --git a/docs/router/framework/react/api/router/RouteMatchType.md b/docs/router/framework/react/api/router/RouteMatchType.md index d4559b0bf1..251c2c031e 100644 --- a/docs/router/framework/react/api/router/RouteMatchType.md +++ b/docs/router/framework/react/api/router/RouteMatchType.md @@ -18,7 +18,6 @@ interface RouteMatch { paramsError: unknown searchError: unknown updatedAt: number - loadPromise?: Promise loaderData?: Route['loaderData'] context: Route['allContext'] search: Route['fullSearchSchema'] diff --git a/packages/react-router/src/Match.tsx b/packages/react-router/src/Match.tsx index 7ec21746e3..0661c6ae3a 100644 --- a/packages/react-router/src/Match.tsx +++ b/packages/react-router/src/Match.tsx @@ -6,7 +6,6 @@ import { getLocationChangeInfo, isNotFound, isRedirect, - pick, rootRouteId, } from '@tanstack/router-core' import { CatchBoundary, ErrorComponent } from './CatchBoundary' @@ -37,7 +36,11 @@ export const Match = React.memo(function MatchImpl({ match, `Could not find match for matchId "${matchId}". Please file an issue!`, ) - return pick(match, ['routeId', 'ssr', '_displayPending']) + return { + routeId: match.routeId, + ssr: match.ssr, + _displayPending: match._displayPending, + } }, structuralSharing: true as any, }) @@ -204,13 +207,13 @@ export const MatchInner = React.memo(function MatchInnerImpl({ return { key, routeId, - match: pick(match, [ - 'id', - 'status', - 'error', - '_forcePending', - '_displayPending', - ]), + match: { + id: match.id, + status: match.status, + error: match.error, + _forcePending: match._forcePending, + _displayPending: match._displayPending, + }, } }, structuralSharing: true as any, @@ -227,11 +230,11 @@ export const MatchInner = React.memo(function MatchInnerImpl({ }, [key, route.options.component, router.options.defaultComponent]) if (match._displayPending) { - throw router.getMatch(match.id)?.displayPendingPromise + throw router.getMatch(match.id)?._nonReactive.displayPendingPromise } if (match._forcePending) { - throw router.getMatch(match.id)?.minPendingPromise + throw router.getMatch(match.id)?._nonReactive.minPendingPromise } // see also hydrate() in packages/router-core/src/ssr/ssr-client.ts @@ -239,31 +242,26 @@ export const MatchInner = React.memo(function MatchInnerImpl({ // We're pending, and if we have a minPendingMs, we need to wait for it const pendingMinMs = route.options.pendingMinMs ?? router.options.defaultPendingMinMs + if (pendingMinMs) { + const routerMatch = router.getMatch(match.id) + if (routerMatch && !routerMatch._nonReactive.minPendingPromise) { + // Create a promise that will resolve after the minPendingMs + if (!router.isServer) { + const minPendingPromise = createControlledPromise() + + Promise.resolve().then(() => { + routerMatch._nonReactive.minPendingPromise = minPendingPromise + }) - if (pendingMinMs && !router.getMatch(match.id)?.minPendingPromise) { - // Create a promise that will resolve after the minPendingMs - if (!router.isServer) { - const minPendingPromise = createControlledPromise() - - Promise.resolve().then(() => { - router.updateMatch(match.id, (prev) => ({ - ...prev, - minPendingPromise, - })) - }) - - setTimeout(() => { - minPendingPromise.resolve() - - // We've handled the minPendingPromise, so we can delete it - router.updateMatch(match.id, (prev) => ({ - ...prev, - minPendingPromise: undefined, - })) - }, pendingMinMs) + setTimeout(() => { + minPendingPromise.resolve() + // We've handled the minPendingPromise, so we can delete it + routerMatch._nonReactive.minPendingPromise = undefined + }, pendingMinMs) + } } } - throw router.getMatch(match.id)?.loadPromise + throw router.getMatch(match.id)?._nonReactive.loadPromise } if (match.status === 'notFound') { @@ -280,7 +278,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({ // false, // 'Tried to render a redirected route match! This is a weird circumstance, please file an issue!', // ) - throw router.getMatch(match.id)?.loadPromise + throw router.getMatch(match.id)?._nonReactive.loadPromise } if (match.status === 'error') { diff --git a/packages/react-router/tests/store-updates-during-navigation.test.tsx b/packages/react-router/tests/store-updates-during-navigation.test.tsx new file mode 100644 index 0000000000..4c18a56815 --- /dev/null +++ b/packages/react-router/tests/store-updates-during-navigation.test.tsx @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { + Link, + Outlet, + RouterProvider, + createRootRoute, + createRoute, + createRouter, + useRouterState, +} from '../src' +import type { RouteComponent } from '../src' + +afterEach(() => { + window.history.replaceState(null, 'root', '/') + cleanup() +}) + +function setup({ RootComponent }: { RootComponent: RouteComponent }) { + const rootRoute = createRootRoute({ + component: RootComponent, + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () => ( + <> +

IndexTitle

+ Posts + + ), + }) + + const postsRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/posts', + beforeLoad: () => new Promise((resolve) => setTimeout(resolve, 100)), + loader: () => new Promise((resolve) => setTimeout(resolve, 100)), + component: () =>

PostsTitle

, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, postsRoute]), + defaultPendingMs: 100, + defaultPendingMinMs: 300, + defaultPendingComponent: () =>

Loading...

, + }) + + return render() +} + +describe('Store updates during navigation', () => { + it("isn't called *too many* times", async () => { + const select = vi.fn() + + setup({ + RootComponent: () => { + useRouterState({ select }) + return + }, + }) + + // navigate to /posts + const link = await waitFor(() => + screen.getByRole('link', { name: 'Posts' }), + ) + const before = select.mock.calls.length + act(() => link.click()) + const title = await waitFor(() => screen.getByText('PostsTitle')) + expect(title).toBeInTheDocument() + const after = select.mock.calls.length + + // This number should be as small as possible to minimize the amount of work + // that needs to be done during a navigation. + // Any change that increases this number should be investigated. + expect(after - before).toBe(19) + }) +}) diff --git a/packages/router-core/src/Matches.ts b/packages/router-core/src/Matches.ts index 8f99e4f688..ea6885d6ef 100644 --- a/packages/router-core/src/Matches.ts +++ b/packages/router-core/src/Matches.ts @@ -136,11 +136,18 @@ export interface RouteMatch< paramsError: unknown searchError: unknown updatedAt: number - loadPromise?: ControlledPromise - /** @internal */ - beforeLoadPromise?: ControlledPromise - /** @internal */ - loaderPromise?: ControlledPromise + _nonReactive: { + /** @internal */ + beforeLoadPromise?: ControlledPromise + /** @internal */ + loaderPromise?: ControlledPromise + /** @internal */ + pendingTimeout?: ReturnType + loadPromise?: ControlledPromise + displayPendingPromise?: Promise + minPendingPromise?: ControlledPromise + dehydrated?: boolean + } loaderData?: TLoaderData /** @internal */ __routeContext: Record @@ -158,12 +165,9 @@ export interface RouteMatch< headers?: Record globalNotFound?: boolean staticData: StaticDataRouteOption - minPendingPromise?: ControlledPromise - pendingTimeout?: ReturnType + /** This attribute is not reactive */ ssr?: boolean | 'data-only' - _dehydrated?: boolean _forcePending?: boolean - displayPendingPromise?: Promise _displayPending?: boolean } diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index f6bf304292..a49531853c 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1285,6 +1285,9 @@ export class RouterCore< error: undefined, paramsError: parseErrors[index], __routeContext: {}, + _nonReactive: { + loadPromise: createControlledPromise(), + }, __beforeLoadContext: undefined, context: {}, abortController: new AbortController(), @@ -1300,7 +1303,6 @@ export class RouterCore< headScripts: undefined, meta: undefined, staticData: route.options.staticData || {}, - loadPromise: createControlledPromise(), fullPath: route.fullPath, } } @@ -1388,13 +1390,8 @@ export class RouterCore< if (!match) return match.abortController.abort() - this.updateMatch(id, (prev) => { - clearTimeout(prev.pendingTimeout) - return { - ...prev, - pendingTimeout: undefined, - } - }) + match._nonReactive.pendingTimeout = undefined + clearTimeout(match._nonReactive.pendingTimeout) } cancelMatches = () => { @@ -2133,8 +2130,10 @@ export class RouterCore< } } - match.beforeLoadPromise?.resolve() - match.loaderPromise?.resolve() + match._nonReactive.beforeLoadPromise?.resolve() + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.beforeLoadPromise = undefined + match._nonReactive.loaderPromise = undefined updateMatch(match.id, (prev) => ({ ...prev, @@ -2145,15 +2144,13 @@ export class RouterCore< : 'error', isFetching: false, error: err, - beforeLoadPromise: undefined, - loaderPromise: undefined, })) if (!(err as any).routeId) { ;(err as any).routeId = match.routeId } - match.loadPromise?.resolve() + match._nonReactive.loadPromise?.resolve() if (isRedirect(err)) { rendered = true @@ -2173,7 +2170,7 @@ export class RouterCore< const shouldSkipLoader = (matchId: string) => { const match = this.getMatch(matchId)! // upon hydration, we skip the loader if the match has been dehydrated on the server - if (!this.isServer && match._dehydrated) { + if (!this.isServer && match._nonReactive.dehydrated) { return true } @@ -2216,8 +2213,9 @@ export class RouterCore< } updateMatch(matchId, (prev) => { - prev.beforeLoadPromise?.resolve() - prev.loadPromise?.resolve() + prev._nonReactive.beforeLoadPromise?.resolve() + prev._nonReactive.beforeLoadPromise = undefined + prev._nonReactive.loadPromise?.resolve() return { ...prev, @@ -2226,7 +2224,6 @@ export class RouterCore< isFetching: false, updatedAt: Date.now(), abortController: new AbortController(), - beforeLoadPromise: undefined, } }) } @@ -2296,10 +2293,7 @@ export class RouterCore< } } } - updateMatch(matchId, (prev) => ({ - ...prev, - ssr, - })) + existingMatch.ssr = ssr } if (shouldSkipLoader(matchId)) { @@ -2321,9 +2315,10 @@ export class RouterCore< let executeBeforeLoad = true const setupPendingTimeout = () => { + const match = this.getMatch(matchId)! if ( shouldPending && - this.getMatch(matchId)!.pendingTimeout === undefined + match._nonReactive.pendingTimeout === undefined ) { const pendingTimeout = setTimeout(() => { try { @@ -2332,22 +2327,19 @@ export class RouterCore< triggerOnReady() } catch {} }, pendingMs) - updateMatch(matchId, (prev) => ({ - ...prev, - pendingTimeout, - })) + match._nonReactive.pendingTimeout = pendingTimeout } } if ( // If we are in the middle of a load, either of these will be present // (not to be confused with `loadPromise`, which is always defined) - existingMatch.beforeLoadPromise || - existingMatch.loaderPromise + existingMatch._nonReactive.beforeLoadPromise || + existingMatch._nonReactive.loaderPromise ) { setupPendingTimeout() // Wait for the beforeLoad to resolve before we continue - await existingMatch.beforeLoadPromise + await existingMatch._nonReactive.beforeLoadPromise const match = this.getMatch(matchId)! if (match.status === 'error') { executeBeforeLoad = true @@ -2361,17 +2353,15 @@ export class RouterCore< if (executeBeforeLoad) { // If we are not in the middle of a load OR the previous load failed, start it try { - updateMatch(matchId, (prev) => { - // explicitly capture the previous loadPromise - const prevLoadPromise = prev.loadPromise - return { - ...prev, - loadPromise: createControlledPromise(() => { - prevLoadPromise?.resolve() - }), - beforeLoadPromise: createControlledPromise(), - } - }) + const match = this.getMatch(matchId)! + match._nonReactive.beforeLoadPromise = + createControlledPromise() + // explicitly capture the previous loadPromise + const prevLoadPromise = match._nonReactive.loadPromise + match._nonReactive.loadPromise = + createControlledPromise(() => { + prevLoadPromise?.resolve() + }) const { paramsError, searchError } = this.getMatch(matchId)! @@ -2453,11 +2443,11 @@ export class RouterCore< } updateMatch(matchId, (prev) => { - prev.beforeLoadPromise?.resolve() + prev._nonReactive.beforeLoadPromise?.resolve() + prev._nonReactive.beforeLoadPromise = undefined return { ...prev, - beforeLoadPromise: undefined, isFetching: false, } }) @@ -2507,8 +2497,8 @@ export class RouterCore< const potentialPendingMinPromise = async () => { const latestMatch = this.getMatch(matchId)! - if (latestMatch.minPendingPromise) { - await latestMatch.minPendingPromise + if (latestMatch._nonReactive.minPendingPromise) { + await latestMatch._nonReactive.minPendingPromise } } @@ -2524,7 +2514,7 @@ export class RouterCore< } } // there is a loaderPromise, so we are in the middle of a load - else if (prevMatch.loaderPromise) { + else if (prevMatch._nonReactive.loaderPromise) { // do not block if we already have stale data we can show // but only if the ongoing load is not a preload since error handling is different for preloads // and we don't want to swallow errors @@ -2535,7 +2525,7 @@ export class RouterCore< ) { return this.getMatch(matchId)! } - await prevMatch.loaderPromise + await prevMatch._nonReactive.loaderPromise const match = this.getMatch(matchId)! if (match.error) { handleRedirectAndNotFound(match, match.error) @@ -2592,13 +2582,16 @@ export class RouterCore< ? shouldReloadOption(getLoaderContext()) : shouldReloadOption - updateMatch(matchId, (prev) => ({ - ...prev, - loaderPromise: createControlledPromise(), - preload: - !!preload && - !this.state.matches.some((d) => d.id === matchId), - })) + updateMatch(matchId, (prev) => { + prev._nonReactive.loaderPromise = + createControlledPromise() + return { + ...prev, + preload: + !!preload && + !this.state.matches.some((d) => d.id === matchId), + } + }) const runLoader = async () => { try { @@ -2682,11 +2675,13 @@ export class RouterCore< } catch (err) { const head = await executeHead() - updateMatch(matchId, (prev) => ({ - ...prev, - loaderPromise: undefined, - ...head, - })) + updateMatch(matchId, (prev) => { + prev._nonReactive.loaderPromise = undefined + return { + ...prev, + ...head, + } + }) handleRedirectAndNotFound(this.getMatch(matchId)!, err) } } @@ -2703,14 +2698,10 @@ export class RouterCore< ;(async () => { try { await runLoader() - const { loaderPromise, loadPromise } = - this.getMatch(matchId)! - loaderPromise?.resolve() - loadPromise?.resolve() - updateMatch(matchId, (prev) => ({ - ...prev, - loaderPromise: undefined, - })) + const match = this.getMatch(matchId)! + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.loadPromise?.resolve() + match._nonReactive.loaderPromise = undefined } catch (err) { if (isRedirect(err)) { await this.navigate(err.options) @@ -2734,25 +2725,23 @@ export class RouterCore< } } if (!loaderIsRunningAsync) { - const { loaderPromise, loadPromise } = - this.getMatch(matchId)! - loaderPromise?.resolve() - loadPromise?.resolve() + const match = this.getMatch(matchId)! + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.loadPromise?.resolve() } updateMatch(matchId, (prev) => { - clearTimeout(prev.pendingTimeout) + clearTimeout(prev._nonReactive.pendingTimeout) + prev._nonReactive.pendingTimeout = undefined + if (!loaderIsRunningAsync) + prev._nonReactive.loaderPromise = undefined + prev._nonReactive.dehydrated = undefined return { ...prev, isFetching: loaderIsRunningAsync ? prev.isFetching : false, - loaderPromise: loaderIsRunningAsync - ? prev.loaderPromise - : undefined, invalid: false, - pendingTimeout: undefined, - _dehydrated: undefined, } }) return this.getMatch(matchId)! diff --git a/packages/router-core/src/ssr/ssr-client.ts b/packages/router-core/src/ssr/ssr-client.ts index a2b446be62..668a42938f 100644 --- a/packages/router-core/src/ssr/ssr-client.ts +++ b/packages/router-core/src/ssr/ssr-client.ts @@ -20,17 +20,16 @@ export interface TsrSsrGlobal { } function hydrateMatch( + match: AnyRouteMatch, deyhydratedMatch: DehydratedMatch, -): Partial { - return { - id: deyhydratedMatch.i, - __beforeLoadContext: deyhydratedMatch.b, - loaderData: deyhydratedMatch.l, - status: deyhydratedMatch.s, - ssr: deyhydratedMatch.ssr, - updatedAt: deyhydratedMatch.u, - error: deyhydratedMatch.e, - } +): void { + match.id = deyhydratedMatch.i + match.__beforeLoadContext = deyhydratedMatch.b + match.loaderData = deyhydratedMatch.l + match.status = deyhydratedMatch.s + match.ssr = deyhydratedMatch.ssr + match.updatedAt = deyhydratedMatch.u + match.error = deyhydratedMatch.e } export interface DehydratedMatch { i: MakeRouteMatch['id'] @@ -80,17 +79,19 @@ export async function hydrate(router: AnyRouter): Promise { route.options.pendingMinMs ?? router.options.defaultPendingMinMs if (pendingMinMs) { const minPendingPromise = createControlledPromise() - match.minPendingPromise = minPendingPromise + match._nonReactive.minPendingPromise = minPendingPromise match._forcePending = true setTimeout(() => { minPendingPromise.resolve() // We've handled the minPendingPromise, so we can delete it - router.updateMatch(match.id, (prev) => ({ - ...prev, - minPendingPromise: undefined, - _forcePending: undefined, - })) + router.updateMatch(match.id, (prev) => { + prev._nonReactive.minPendingPromise = undefined + return { + ...prev, + _forcePending: undefined, + } + }) }, pendingMinMs) } } @@ -103,17 +104,14 @@ export async function hydrate(router: AnyRouter): Promise { (d) => d.i === match.id, ) if (!dehydratedMatch) { - Object.assign(match, { dehydrated: false, ssr: false }) + match._nonReactive.dehydrated = false + match.ssr = false return } - Object.assign(match, hydrateMatch(dehydratedMatch)) + hydrateMatch(match, dehydratedMatch) - if (match.ssr === false) { - match._dehydrated = false - } else { - match._dehydrated = true - } + match._nonReactive.dehydrated = match.ssr !== false if (match.ssr === 'data-only' || match.ssr === false) { if (firstNonSsrMatchIndex === undefined) { @@ -186,11 +184,11 @@ export async function hydrate(router: AnyRouter): Promise { const isSpaMode = matches[matches.length - 1]!.id !== lastMatchId const hasSsrFalseMatches = matches.some((m) => m.ssr === false) - // all matches have data from the server and we are not in SPA mode so we don't need to kick of router.load() + // all matches have data from the server and we are not in SPA mode so we don't need to kick of router.load() if (!hasSsrFalseMatches && !isSpaMode) { matches.forEach((match) => { - // remove the _dehydrate flag since we won't run router.load() which would remove it - match._dehydrated = undefined + // remove the dehydrated flag since we won't run router.load() which would remove it + match._nonReactive.dehydrated = undefined }) return routeChunkPromise } @@ -213,7 +211,7 @@ export async function hydrate(router: AnyRouter): Promise { setMatchForcePending(match) match._displayPending = true - match.displayPendingPromise = loadPromise + match._nonReactive.displayPendingPromise = loadPromise loadPromise.then(() => { batch(() => { diff --git a/packages/solid-router/src/Match.tsx b/packages/solid-router/src/Match.tsx index ccc8cf5147..e039a2c66b 100644 --- a/packages/solid-router/src/Match.tsx +++ b/packages/solid-router/src/Match.tsx @@ -6,7 +6,6 @@ import { getLocationChangeInfo, isNotFound, isRedirect, - pick, rootRouteId, } from '@tanstack/router-core' import { Dynamic } from 'solid-js/web' @@ -30,7 +29,11 @@ export const Match = (props: { matchId: string }) => { match, `Could not find match for matchId "${props.matchId}". Please file an issue!`, ) - return pick(match, ['routeId', 'ssr', '_displayPending']) + return { + routeId: match.routeId, + ssr: match.ssr, + _displayPending: match._displayPending, + } }, }) @@ -200,13 +203,13 @@ export const MatchInner = (props: { matchId: string }): any => { return { key, routeId, - match: pick(match, [ - 'id', - 'status', - 'error', - '_forcePending', - '_displayPending', - ]), + match: { + id: match.id, + status: match.status, + error: match.error, + _forcePending: match._forcePending, + _displayPending: match._displayPending, + }, } }, }) @@ -232,7 +235,8 @@ export const MatchInner = (props: { matchId: string }): any => { {(_) => { const [displayPendingResult] = Solid.createResource( - () => router.getMatch(match().id)?.displayPendingPromise, + () => + router.getMatch(match().id)?._nonReactive.displayPendingPromise, ) return <>{displayPendingResult()} @@ -241,7 +245,7 @@ export const MatchInner = (props: { matchId: string }): any => { {(_) => { const [minPendingResult] = Solid.createResource( - () => router.getMatch(match().id)?.minPendingPromise, + () => router.getMatch(match().id)?._nonReactive.minPendingPromise, ) return <>{minPendingResult()} @@ -252,33 +256,29 @@ export const MatchInner = (props: { matchId: string }): any => { const pendingMinMs = route().options.pendingMinMs ?? router.options.defaultPendingMinMs - if (pendingMinMs && !router.getMatch(match().id)?.minPendingPromise) { - // Create a promise that will resolve after the minPendingMs - if (!router.isServer) { - const minPendingPromise = createControlledPromise() - - Promise.resolve().then(() => { - router.updateMatch(match().id, (prev) => ({ - ...prev, - minPendingPromise, - })) - }) - - setTimeout(() => { - minPendingPromise.resolve() - - // We've handled the minPendingPromise, so we can delete it - router.updateMatch(match().id, (prev) => ({ - ...prev, - minPendingPromise: undefined, - })) - }, pendingMinMs) + if (pendingMinMs) { + const routerMatch = router.getMatch(match().id) + if (routerMatch && !routerMatch._nonReactive.minPendingPromise) { + // Create a promise that will resolve after the minPendingMs + if (!router.isServer) { + const minPendingPromise = createControlledPromise() + + Promise.resolve().then(() => { + routerMatch._nonReactive.minPendingPromise = minPendingPromise + }) + + setTimeout(() => { + minPendingPromise.resolve() + // We've handled the minPendingPromise, so we can delete it + routerMatch._nonReactive.minPendingPromise = undefined + }, pendingMinMs) + } } } const [loaderResult] = Solid.createResource(async () => { await new Promise((r) => setTimeout(r, 0)) - return router.getMatch(match().id)?.loadPromise + return router.getMatch(match().id)?._nonReactive.loadPromise }) return <>{loaderResult()} @@ -297,7 +297,7 @@ export const MatchInner = (props: { matchId: string }): any => { const [loaderResult] = Solid.createResource(async () => { await new Promise((r) => setTimeout(r, 0)) - return router.getMatch(match().id)?.loadPromise + return router.getMatch(match().id)?._nonReactive.loadPromise }) return <>{loaderResult()}