fix(router-core): avoid preload cleanup crashes after invalidation

.changeset/moody-kings-travel.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/router-core': patch
+---
+
+evicting cache during preload does not error, return early

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<void> => {
   if (!inner.rendered) {
     inner.rendered = true
@@ -788,6 +822,8 @@ const loadRouteMatch = async (
   matchPromises: Array<Promise<AnyRouteMatch>>,
   index: number,
 ): Promise<AnyRouteMatch> => {
+  let cleanupMatch: AnyRouteMatch | undefined
+
   async function handleLoader(
     preload: boolean,
     prevMatch: AnyRouteMatch,
@@ -834,18 +870,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) {
@@ -867,18 +904,19 @@ 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)
 
     if (isServer ?? inner.router.isServer) {
       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 = getMatchOrThrowCancelled(inner, matchId, cleanupMatch)
+
+    // This is where all of the stale-while-revalidate magic happens
     const activeIdAtIndex = inner.router.stores.matchesId.state[index]
     const activeAtIndex =
       (activeIdAtIndex &&
@@ -906,7 +944,9 @@ const loadRouteMatch = async (
         return prevMatch
       }
       await prevMatch._nonReactive.loaderPromise
-      const match = inner.router.getMatch(matchId)!
+      const match = getMatchOrThrowCancelled(inner, matchId, cleanupMatch)
+      cleanupMatch = match
+
       const error = match._nonReactive.error || match.error
       if (error) {
         handleRedirectAndNotFound(inner, match, error)
@@ -924,7 +964,9 @@ const loadRouteMatch = async (
     } else {
       const nextPreload =
         preload && !inner.router.stores.activeMatchStoresById.has(matchId)
-      const match = inner.router.getMatch(matchId)!
+      const match = getMatchOrThrowCancelled(inner, matchId, cleanupMatch)
+      cleanupMatch = match
+
       match._nonReactive.loaderPromise = createControlledPromise<void>()
       if (nextPreload !== match.preload) {
         inner.updateMatch(matchId, (prev) => ({
@@ -936,7 +978,7 @@ const loadRouteMatch = async (
       await handleLoader(preload, prevMatch, previousRouteMatchId, match, route)
     }
   }
-  const match = inner.router.getMatch(matchId)!
+  const match = getMatchOrThrowCancelled(inner, matchId, cleanupMatch)
   if (!loaderIsRunningAsync) {
     match._nonReactive.loaderPromise?.resolve()
     match._nonReactive.loadPromise?.resolve()
@@ -955,10 +997,8 @@ const loadRouteMatch = async (
       isFetching: nextIsFetching,
       invalid: false,
     }))
-    return inner.router.getMatch(matchId)!
-  } else {
-    return match
   }
+  return match
 }
 
 export async function loadMatches(arg: {
@@ -1024,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))
@@ -1038,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
       }
@@ -1048,6 +1093,10 @@ export async function loadMatches(arg: {
       }
     }
 
+    if (firstCancelledMatch) {
+      throw firstCancelledMatch
+    }
+
     if (firstUnhandledRejection !== undefined) {
       throw firstUnhandledRejection
     }

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

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()