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

Author: Sheraff

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

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