From ce450adb9a402d0a53f4d328f58c77c07d61e72d Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sat, 21 Mar 2026 11:55:23 +0100 Subject: [PATCH 1/3] fix(router-core): avoid preload cleanup crashes after invalidation --- packages/router-core/src/load-matches.ts | 38 +++++-- packages/router-core/tests/load.test.ts | 120 +++++++++++++++++++++++ 2 files changed, 148 insertions(+), 10 deletions(-) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 84071950781..a27f1861844 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -834,18 +834,19 @@ const loadRouteMatch = async ( shouldReloadInBackground ) { loaderIsRunningAsync = true + const matchForCleanup = prevMatch ;(async () => { try { await runLoader(inner, matchPromises, matchId, index, route) - const match = inner.router.getMatch(matchId)! - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.loadPromise?.resolve() - match._nonReactive.loaderPromise = undefined - match._nonReactive.loadPromise = undefined } catch (err) { if (isRedirect(err)) { await inner.router.navigate(err.options) } + } finally { + matchForCleanup._nonReactive.loaderPromise?.resolve() + matchForCleanup._nonReactive.loadPromise?.resolve() + matchForCleanup._nonReactive.loaderPromise = undefined + matchForCleanup._nonReactive.loadPromise = undefined } })() } else if (status !== 'success' || loaderShouldRunAsync) { @@ -878,7 +879,12 @@ const loadRouteMatch = async ( return inner.router.getMatch(matchId)! } } else { - const prevMatch = inner.router.getMatch(matchId)! // This is where all of the stale-while-revalidate magic happens + const prevMatch = inner.router.getMatch(matchId) + if (!prevMatch) { + return inner.matches[index]! + } + + // This is where all of the stale-while-revalidate magic happens const activeIdAtIndex = inner.router.stores.matchesId.state[index] const activeAtIndex = (activeIdAtIndex && @@ -906,7 +912,11 @@ const loadRouteMatch = async ( return prevMatch } await prevMatch._nonReactive.loaderPromise - const match = inner.router.getMatch(matchId)! + const match = inner.router.getMatch(matchId) + if (!match) { + return inner.matches[index]! + } + const error = match._nonReactive.error || match.error if (error) { handleRedirectAndNotFound(inner, match, error) @@ -924,7 +934,11 @@ const loadRouteMatch = async ( } else { const nextPreload = preload && !inner.router.stores.activeMatchStoresById.has(matchId) - const match = inner.router.getMatch(matchId)! + const match = inner.router.getMatch(matchId) + if (!match) { + return inner.matches[index]! + } + match._nonReactive.loaderPromise = createControlledPromise() if (nextPreload !== match.preload) { inner.updateMatch(matchId, (prev) => ({ @@ -936,7 +950,11 @@ const loadRouteMatch = async ( await handleLoader(preload, prevMatch, previousRouteMatchId, match, route) } } - const match = inner.router.getMatch(matchId)! + const match = inner.router.getMatch(matchId) + if (!match) { + return inner.matches[index]! + } + if (!loaderIsRunningAsync) { match._nonReactive.loaderPromise?.resolve() match._nonReactive.loadPromise?.resolve() @@ -955,7 +973,7 @@ const loadRouteMatch = async ( isFetching: nextIsFetching, invalid: false, })) - return inner.router.getMatch(matchId)! + return inner.router.getMatch(matchId) ?? match } else { return match } diff --git a/packages/router-core/tests/load.test.ts b/packages/router-core/tests/load.test.ts index 142fea1fe1b..e6eacd88578 100644 --- a/packages/router-core/tests/load.test.ts +++ b/packages/router-core/tests/load.test.ts @@ -381,6 +381,126 @@ describe('loader skip or exec', () => { expect(loader).toHaveBeenCalledTimes(1) }) + test('does not error if cache gc clears an in-flight preload', async () => { + let resolveLoader: ((value: { ok: true }) => void) | undefined + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}) + + const loader: Loader = vi.fn( + () => + new Promise<{ ok: true }>((resolve) => { + resolveLoader = resolve + }), + ) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader, + preloadGcTime: 0, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute]), + history: createMemoryHistory(), + defaultPreloadGcTime: 0, + }) + + const preloadPromise = router.preloadRoute({ to: '/foo' }) + await Promise.resolve() + + expect( + router.stores.cachedMatchesSnapshot.state.some( + (match) => match.routeId === fooRoute.id, + ), + ).toBe(true) + + router.clearExpiredCache() + + expect( + router.stores.cachedMatchesSnapshot.state.some( + (match) => match.routeId === fooRoute.id, + ), + ).toBe(false) + + resolveLoader?.({ ok: true }) + + await preloadPromise + // the route load won't throw, but it will log errors to the console if any + expect(consoleErrorSpy).not.toHaveBeenCalled() + + expect( + router.stores.cachedMatchesSnapshot.state.some( + (match) => match.routeId === fooRoute.id, + ), + ).toBe(false) + consoleErrorSpy.mockRestore() + }) + + test('does not error when invalidate clears an in-flight preload', async () => { + let resolveFooLoader: ((value: { ok: true }) => void) | undefined + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}) + + const fooLoader: Loader = vi.fn( + () => + new Promise<{ ok: true }>((resolve) => { + resolveFooLoader = resolve + }), + ) + const barLoader: Loader = vi.fn(() => ({ ok: true })) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader: fooLoader, + preloadGcTime: 0, + }) + const barRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bar', + loader: barLoader, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute, barRoute]), + history: createMemoryHistory(), + defaultPreloadGcTime: 0, + }) + + await router.navigate({ to: '/bar' }) + + const preloadPromise = router.preloadRoute({ to: '/foo' }) + await Promise.resolve() + + expect( + router.stores.cachedMatchesSnapshot.state.some( + (match) => match.routeId === fooRoute.id, + ), + ).toBe(true) + + const invalidatePromise = router.invalidate() + await Promise.resolve() + + resolveFooLoader?.({ ok: true }) + + await Promise.all([preloadPromise, invalidatePromise]) + + expect(barLoader).toHaveBeenCalledTimes(2) + // the route load won't throw, but it will log errors to the console if any + expect(consoleErrorSpy).not.toHaveBeenCalled() + expect( + router.stores.cachedMatchesSnapshot.state.some( + (match) => match.routeId === fooRoute.id, + ), + ).toBe(false) + consoleErrorSpy.mockRestore() + }) + test('exec if rejected preload (notFound)', async () => { const loader: Loader = vi.fn(async ({ preload }) => { if (preload) throw notFound() From 911a46e1d83e6d942357516a64748ab5804c3902 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sat, 21 Mar 2026 12:02:46 +0100 Subject: [PATCH 2/3] changeset --- .changeset/moody-kings-travel.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/moody-kings-travel.md diff --git a/.changeset/moody-kings-travel.md b/.changeset/moody-kings-travel.md new file mode 100644 index 00000000000..fd4cc1e35ef --- /dev/null +++ b/.changeset/moody-kings-travel.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +evicting cache during preload does not error, return early From d86b13dc52ad789085817370ff83edc563bdd05f Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sat, 21 Mar 2026 17:05:19 +0100 Subject: [PATCH 3/3] error based solution --- packages/router-core/src/load-matches.ts | 79 +++++++++++++++++------- packages/router-core/src/router.ts | 10 ++- packages/router-core/tests/load.test.ts | 8 ++- 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index a27f1861844..ba22b44ddc6 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -37,6 +37,40 @@ type InnerLoadContext = { sync?: boolean } +const reason = 'Match removed before complete load' +class MatchLoadCancelledError extends Error { + constructor() { + super(reason) + this.name = 'MatchCancel' + } +} + +export const isMatchLoadCancelledError = ( + err: unknown, +): err is MatchLoadCancelledError => err instanceof MatchLoadCancelledError + +const getMatchOrThrowCancelled = ( + inner: InnerLoadContext, + matchId: string, + cleanupMatch?: AnyRouteMatch, +): AnyRouteMatch => { + const match = inner.router.getMatch(matchId) + if (match) return match as AnyRouteMatch + if (cleanupMatch) { + const s = cleanupMatch._nonReactive + s.beforeLoadPromise?.resolve() + s.loaderPromise?.resolve() + s.loadPromise?.resolve() + cleanupMatch.abortController.abort(reason) + clearTimeout(s.pendingTimeout) + s.beforeLoadPromise = undefined + s.loaderPromise = undefined + s.loadPromise = undefined + s.pendingTimeout = undefined + } + throw new MatchLoadCancelledError() +} + const triggerOnReady = (inner: InnerLoadContext): void | Promise => { if (!inner.rendered) { inner.rendered = true @@ -788,6 +822,8 @@ const loadRouteMatch = async ( matchPromises: Array>, index: number, ): Promise => { + let cleanupMatch: AnyRouteMatch | undefined + async function handleLoader( preload: boolean, prevMatch: AnyRouteMatch, @@ -868,10 +904,9 @@ const loadRouteMatch = async ( inner.router.options.defaultStaleReloadMode) !== 'blocking' if (shouldSkipLoader(inner, matchId)) { - const match = inner.router.getMatch(matchId) - if (!match) { - return inner.matches[index]! - } + const match = getMatchOrThrowCancelled(inner, matchId, cleanupMatch) + + cleanupMatch = match syncMatchContext(inner, matchId, index) @@ -879,10 +914,7 @@ const loadRouteMatch = async ( return inner.router.getMatch(matchId)! } } else { - const prevMatch = inner.router.getMatch(matchId) - if (!prevMatch) { - return inner.matches[index]! - } + const prevMatch = getMatchOrThrowCancelled(inner, matchId, cleanupMatch) // This is where all of the stale-while-revalidate magic happens const activeIdAtIndex = inner.router.stores.matchesId.state[index] @@ -912,10 +944,8 @@ const loadRouteMatch = async ( return prevMatch } await prevMatch._nonReactive.loaderPromise - const match = inner.router.getMatch(matchId) - if (!match) { - return inner.matches[index]! - } + const match = getMatchOrThrowCancelled(inner, matchId, cleanupMatch) + cleanupMatch = match const error = match._nonReactive.error || match.error if (error) { @@ -934,10 +964,8 @@ const loadRouteMatch = async ( } else { const nextPreload = preload && !inner.router.stores.activeMatchStoresById.has(matchId) - const match = inner.router.getMatch(matchId) - if (!match) { - return inner.matches[index]! - } + const match = getMatchOrThrowCancelled(inner, matchId, cleanupMatch) + cleanupMatch = match match._nonReactive.loaderPromise = createControlledPromise() if (nextPreload !== match.preload) { @@ -950,11 +978,7 @@ const loadRouteMatch = async ( await handleLoader(preload, prevMatch, previousRouteMatchId, match, route) } } - const match = inner.router.getMatch(matchId) - if (!match) { - return inner.matches[index]! - } - + const match = getMatchOrThrowCancelled(inner, matchId, cleanupMatch) if (!loaderIsRunningAsync) { match._nonReactive.loaderPromise?.resolve() match._nonReactive.loadPromise?.resolve() @@ -973,10 +997,8 @@ const loadRouteMatch = async ( isFetching: nextIsFetching, invalid: false, })) - return inner.router.getMatch(matchId) ?? match - } else { - return match } + return match } export async function loadMatches(arg: { @@ -1042,6 +1064,7 @@ export async function loadMatches(arg: { let firstNotFound: NotFoundError | undefined let firstUnhandledRejection: unknown + let firstCancelledMatch: MatchLoadCancelledError | undefined for (let i = 0; i < maxIndexExclusive; i++) { matchPromises.push(loadRouteMatch(inner, matchPromises, i)) @@ -1056,6 +1079,10 @@ export async function loadMatches(arg: { if (result.status !== 'rejected') continue const reason = result.reason + if (isMatchLoadCancelledError(reason)) { + firstCancelledMatch ??= reason + continue + } if (isRedirect(reason)) { throw reason } @@ -1066,6 +1093,10 @@ export async function loadMatches(arg: { } } + if (firstCancelledMatch) { + throw firstCancelledMatch + } + if (firstUnhandledRejection !== undefined) { throw firstUnhandledRejection } diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 965fefda339..3eb79c47cff 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -34,7 +34,12 @@ import { setupScrollRestoration } from './scroll-restoration' import { defaultParseSearch, defaultStringifySearch } from './searchParams' import { rootRouteId } from './root' import { isRedirect, redirect } from './redirect' -import { loadMatches, loadRouteChunk, routeNeedsPreload } from './load-matches' +import { + isMatchLoadCancelledError, + loadMatches, + loadRouteChunk, + routeNeedsPreload, +} from './load-matches' import { composeRewrites, executeRewriteInput, @@ -2852,6 +2857,9 @@ export class RouterCore< return matches } catch (err) { + if (isMatchLoadCancelledError(err)) { + return undefined + } if (isRedirect(err)) { if (err.options.reloadDocument) { return undefined diff --git a/packages/router-core/tests/load.test.ts b/packages/router-core/tests/load.test.ts index e6eacd88578..ddcf3016e4d 100644 --- a/packages/router-core/tests/load.test.ts +++ b/packages/router-core/tests/load.test.ts @@ -427,7 +427,7 @@ describe('loader skip or exec', () => { resolveLoader?.({ ok: true }) - await preloadPromise + await expect(preloadPromise).resolves.toBeUndefined() // the route load won't throw, but it will log errors to the console if any expect(consoleErrorSpy).not.toHaveBeenCalled() @@ -488,9 +488,13 @@ describe('loader skip or exec', () => { resolveFooLoader?.({ ok: true }) - await Promise.all([preloadPromise, invalidatePromise]) + const [preloadResult] = await Promise.all([ + preloadPromise, + invalidatePromise, + ]) expect(barLoader).toHaveBeenCalledTimes(2) + expect(preloadResult).toBeUndefined() // the route load won't throw, but it will log errors to the console if any expect(consoleErrorSpy).not.toHaveBeenCalled() expect(