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 diff --git a/packages/react-router/src/Match.tsx b/packages/react-router/src/Match.tsx index 36b41ef1240..c860fc7274a 100644 --- a/packages/react-router/src/Match.tsx +++ b/packages/react-router/src/Match.tsx @@ -101,6 +101,29 @@ type MatchViewState = { parentRouteId: string | undefined } +function getMatchNonReactivePromise( + router: ReturnType, + match: { + id: string + _nonReactive: { + displayPendingPromise?: Promise + minPendingPromise?: Promise + loadPromise?: Promise + } + }, + key: 'displayPendingPromise' | 'minPendingPromise' | 'loadPromise', +) { + const promise = + router.getMatch(match.id)?._nonReactive[key] ?? match._nonReactive[key] + + invariant( + promise, + `Missing suspense promise '${key}' for match '${match.id}'`, + ) + + return promise +} + function MatchView({ router, matchId, @@ -270,15 +293,15 @@ export const MatchInner = React.memo(function MatchInnerImpl({ const out = Comp ? : if (match._displayPending) { - throw router.getMatch(match.id)?._nonReactive.displayPendingPromise + throw getMatchNonReactivePromise(router, match, 'displayPendingPromise') } if (match._forcePending) { - throw router.getMatch(match.id)?._nonReactive.minPendingPromise + throw getMatchNonReactivePromise(router, match, 'minPendingPromise') } if (match.status === 'pending') { - throw router.getMatch(match.id)?._nonReactive.loadPromise + throw getMatchNonReactivePromise(router, match, 'loadPromise') } if (match.status === 'notFound') { @@ -288,7 +311,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({ if (match.status === 'redirected') { invariant(isRedirect(match.error), 'Expected a redirect error') - throw router.getMatch(match.id)?._nonReactive.loadPromise + throw getMatchNonReactivePromise(router, match, 'loadPromise') } if (match.status === 'error') { @@ -351,11 +374,11 @@ export const MatchInner = React.memo(function MatchInnerImpl({ }, [key, route.options.component, router.options.defaultComponent]) if (match._displayPending) { - throw router.getMatch(match.id)?._nonReactive.displayPendingPromise + throw getMatchNonReactivePromise(router, match, 'displayPendingPromise') } if (match._forcePending) { - throw router.getMatch(match.id)?._nonReactive.minPendingPromise + throw getMatchNonReactivePromise(router, match, 'minPendingPromise') } // see also hydrate() in packages/router-core/src/ssr/ssr-client.ts @@ -380,7 +403,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({ } } } - throw router.getMatch(match.id)?._nonReactive.loadPromise + throw getMatchNonReactivePromise(router, match, 'loadPromise') } if (match.status === 'notFound') { @@ -397,7 +420,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)?._nonReactive.loadPromise + throw getMatchNonReactivePromise(router, match, 'loadPromise') } if (match.status === 'error') { diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 84071950781..4c1bd814a3a 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -250,16 +250,23 @@ const handleSerialError = ( } } +const matchIsAvailable = ( + match: AnyRouteMatch | undefined, +): match is AnyRouteMatch => + Boolean(match && !match.abortController.signal.aborted) + const isBeforeLoadSsr = ( inner: InnerLoadContext, matchId: string, index: number, route: AnyRoute, ): void | Promise => { - const existingMatch = inner.router.getMatch(matchId)! + const existingMatch = inner.router.getMatch(matchId) + if (!existingMatch) return + const parentMatchId = inner.matches[index - 1]?.id const parentMatch = parentMatchId - ? inner.router.getMatch(parentMatchId)! + ? inner.router.getMatch(parentMatchId) : undefined // in SPA mode, only SSR the root route @@ -359,7 +366,8 @@ const preBeforeLoadSetup = ( matchId: string, route: AnyRoute, ): void | Promise => { - const existingMatch = inner.router.getMatch(matchId)! + const existingMatch = inner.router.getMatch(matchId) + if (!existingMatch) return // 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) @@ -372,7 +380,9 @@ const preBeforeLoadSetup = ( setupPendingTimeout(inner, matchId, route, existingMatch) const then = () => { - const match = inner.router.getMatch(matchId)! + const match = inner.router.getMatch(matchId) + if (!match) return + if ( match.preload && (match.status === 'redirected' || match.status === 'notFound') @@ -393,7 +403,8 @@ const executeBeforeLoad = ( index: number, route: AnyRoute, ): void | Promise => { - const match = inner.router.getMatch(matchId)! + const match = inner.router.getMatch(matchId) + if (!match) return // explicitly capture the previous loadPromise let prevLoadPromise = match._nonReactive.loadPromise @@ -609,17 +620,16 @@ const executeHead = ( const getLoaderContext = ( inner: InnerLoadContext, matchPromises: Array>, - matchId: string, + match: AnyRouteMatch, index: number, route: AnyRoute, ): LoaderFnContext => { const parentMatchPromise = matchPromises[index - 1] as any - const { params, loaderDeps, abortController, cause } = - inner.router.getMatch(matchId)! + const { params, loaderDeps, abortController, cause } = match const context = buildMatchContext(inner, index) - const preload = resolvePreload(inner, matchId) + const preload = resolvePreload(inner, match.id) return { params, @@ -654,7 +664,8 @@ const runLoader = async ( // before committing to the match and resolving // the loadPromise - const match = inner.router.getMatch(matchId)! + const match = inner.router.getMatch(matchId) + if (!matchIsAvailable(match)) return // Actually run the loader and handle the result try { @@ -667,7 +678,7 @@ const runLoader = async ( const loader = typeof routeLoader === 'function' ? routeLoader : routeLoader?.handler const loaderResult = loader?.( - getLoaderContext(inner, matchPromises, matchId, index, route), + getLoaderContext(inner, matchPromises, match, index, route), ) const loaderResultIsPromise = !!loader && isPromise(loaderResult) @@ -693,6 +704,8 @@ const runLoader = async ( ? await loaderResult : loaderResult + if (match.abortController.signal.aborted) return + handleRedirectAndNotFound( inner, inner.router.getMatch(matchId), @@ -716,6 +729,9 @@ const runLoader = async ( // Last but not least, wait for the the components // to be preloaded before we resolve the match if (route._componentsPromise) await route._componentsPromise + + if (match.abortController.signal.aborted) return + inner.updateMatch(matchId, (prev) => ({ ...prev, error: undefined, @@ -727,6 +743,8 @@ const runLoader = async ( } catch (e) { let error = e + if (match.abortController.signal.aborted) return + if ((error as any)?.name === 'AbortError') { if (match.abortController.signal.aborted) { match._nonReactive.loaderPromise?.resolve() @@ -795,6 +813,8 @@ const loadRouteMatch = async ( match: AnyRouteMatch, route: AnyRoute, ) { + if (match.abortController.signal.aborted) return + const age = Date.now() - prevMatch.updatedAt const staleAge = preload @@ -811,7 +831,7 @@ const loadRouteMatch = async ( const shouldReload = typeof shouldReloadOption === 'function' ? shouldReloadOption( - getLoaderContext(inner, matchPromises, matchId, index, route), + getLoaderContext(inner, matchPromises, match, index, route), ) : shouldReloadOption @@ -834,18 +854,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) { @@ -868,9 +889,7 @@ const loadRouteMatch = async ( if (shouldSkipLoader(inner, matchId)) { const match = inner.router.getMatch(matchId) - if (!match) { - return inner.matches[index]! - } + if (!matchIsAvailable(match)) return inner.matches[index]! syncMatchContext(inner, matchId, index) @@ -878,7 +897,10 @@ 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 (!matchIsAvailable(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 +928,11 @@ const loadRouteMatch = async ( return prevMatch } await prevMatch._nonReactive.loaderPromise - const match = inner.router.getMatch(matchId)! + if (prevMatch.abortController.signal.aborted) return inner.matches[index]! + + const match = inner.router.getMatch(matchId) + if (!matchIsAvailable(match)) return inner.matches[index]! + const error = match._nonReactive.error || match.error if (error) { handleRedirectAndNotFound(inner, match, error) @@ -924,7 +950,9 @@ 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 (!matchIsAvailable(match)) return inner.matches[index]! + match._nonReactive.loaderPromise = createControlledPromise() if (nextPreload !== match.preload) { inner.updateMatch(matchId, (prev) => ({ @@ -936,7 +964,9 @@ const loadRouteMatch = async ( await handleLoader(preload, prevMatch, previousRouteMatchId, match, route) } } - const match = inner.router.getMatch(matchId)! + const match = inner.router.getMatch(matchId) + if (!matchIsAvailable(match)) return inner.matches[index]! + if (!loaderIsRunningAsync) { match._nonReactive.loaderPromise?.resolve() match._nonReactive.loadPromise?.resolve() @@ -955,10 +985,8 @@ const loadRouteMatch = async ( isFetching: nextIsFetching, invalid: false, })) - return inner.router.getMatch(matchId)! - } else { - return match } + return match } export async function loadMatches(arg: { @@ -1114,6 +1142,10 @@ export async function loadMatches(arg: { // lazy notFoundComponent) is loaded before we continue to head execution/render. await loadRouteChunk(boundaryRoute, ['notFoundComponent']) } else if (!inner.preload) { + if (inner.router.stores.location.state.href !== inner.location.href) { + return inner.matches + } + // Clear stale root global-not-found state on normal navigations that do not // throw notFound. This must live here (instead of only in runLoader success) // because the root loader may be skipped when data is still fresh. diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 965fefda339..bf8593d98e6 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1538,6 +1538,9 @@ export class RouterCore< let match: AnyRouteMatch if (existingMatch) { + const shouldResetExistingMatch = + existingMatch.abortController.signal.aborted + match = { ...existingMatch, cause, @@ -1547,6 +1550,31 @@ export class RouterCore< ? nullReplaceEqualDeep(previousMatch.search, preMatchSearch) : nullReplaceEqualDeep(existingMatch.search, preMatchSearch), _strictSearch: strictMatchSearch, + _nonReactive: { + ...existingMatch._nonReactive, + loadPromise: + existingMatch._nonReactive.loadPromise ?? + createControlledPromise(), + }, + ...(shouldResetExistingMatch + ? { + isFetching: false, + abortController: new AbortController(), + _displayPending: undefined, + _forcePending: undefined, + _nonReactive: { + ...existingMatch._nonReactive, + beforeLoadPromise: undefined, + loaderPromise: undefined, + pendingTimeout: undefined, + loadPromise: createControlledPromise(), + displayPendingPromise: undefined, + minPendingPromise: undefined, + dehydrated: undefined, + error: undefined, + }, + } + : undefined), } } else { const status = @@ -1759,7 +1787,11 @@ export class RouterCore< if (!match) return - match.abortController.abort() + match._nonReactive.beforeLoadPromise?.resolve() + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.beforeLoadPromise = undefined + match._nonReactive.loaderPromise = undefined + match.abortController.abort('Cancelled match') clearTimeout(match._nonReactive.pendingTimeout) match._nonReactive.pendingTimeout = undefined } @@ -2760,12 +2792,31 @@ export class RouterCore< } clearCache: ClearCacheFn = (opts) => { + const cachedMatches = this.stores.cachedMatchesSnapshot.state const filter = opts?.filter + + const matchesToClear = + filter !== undefined + ? cachedMatches.filter((match) => + filter(match as MakeRouteMatchUnion), + ) + : cachedMatches + + matchesToClear.forEach((match) => { + if ( + match.status === 'pending' || + match.isFetching || + match._nonReactive.beforeLoadPromise || + match._nonReactive.loaderPromise || + match._nonReactive.loadPromise + ) { + this.cancelMatch(match.id) + } + }) + if (filter !== undefined) { this.stores.setCachedMatches( - this.stores.cachedMatchesSnapshot.state.filter( - (m) => !filter(m as MakeRouteMatchUnion), - ), + cachedMatches.filter((m) => !filter(m as MakeRouteMatchUnion)), ) } else { this.stores.setCachedMatches([]) @@ -2850,6 +2901,10 @@ export class RouterCore< }, }) + if (matches.some((match) => !this.getMatch(match.id))) { + return undefined + } + return matches } catch (err) { if (isRedirect(err)) { diff --git a/packages/router-core/tests/load.test.ts b/packages/router-core/tests/load.test.ts index 142fea1fe1b..ddcf3016e4d 100644 --- a/packages/router-core/tests/load.test.ts +++ b/packages/router-core/tests/load.test.ts @@ -381,6 +381,130 @@ 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 expect(preloadPromise).resolves.toBeUndefined() + // 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 }) + + 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( + 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()