perf(router-core): avoid redundant promises for ready routes

.changeset/tame-banks-wear.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/router-core': patch
+---
+
+Reduce promise allocations during client navigation when loader data and route components are already available.

packages/router-core/src/load-client.ts

@@ -288,7 +288,8 @@ export function waitFor<T>(
     signal.addEventListener('abort', abort, { once: true })
     Promise.resolve(value)
       .then(resolve, reject)
-      .finally(() => signal.removeEventListener('abort', abort))
+      // Both settlement callbacks return normally, so this chain fulfills.
+      .then(() => signal.removeEventListener('abort', abort))
   })
 }
 
@@ -859,53 +860,52 @@ function createLoaderTask(
   if (!loaded) {
     match.isFetching = false
   }
-  const loaderOutcome = reloadFailure
-    ? Promise.resolve(reloadFailure)
-    : !blocking
-      ? Promise.resolve<LoaderOutcome>([SUCCESS, match.loaderData])
-      : loadResource(
+  const outcome =
+    !reloadFailure && blocking
+      ? loadResource(
           router,
           lane,
           match,
           route,
           loader,
           semanticParent,
           options,
+        ).then((result) => {
+          settleInto(match, result, preload)
+          if (result[0 /* kind */] === SUCCESS) {
+            // A settled generation can outlive its lane without keeping unresolved
+            // navigation work alive.
+            if (routeLoader && !options[0 /* controller */].signal.aborted) {
+              cacheLoaderMatch(router, match, plannedCacheMatch)
+            }
+            // A route is renderable only after both its data and normal component
+            // chunk are ready. Its loader data is already available to descendants.
+            if (index >= retainedEnd) {
+              match.status = 'pending'
+            }
+          }
+          return result
+        })
+      : Promise.resolve<LoaderOutcome>(
+          reloadFailure ?? [SUCCESS, match.loaderData],
         )
-  const outcome = loaderOutcome.then((result) => {
-    if (blocking) {
-      settleInto(match, result, preload)
-      if (result[0 /* kind */] === SUCCESS) {
-        // A settled generation can outlive its lane without keeping unresolved
-        // navigation work alive.
-        if (routeLoader && !options[0 /* controller */].signal.aborted) {
-          cacheLoaderMatch(router, match, plannedCacheMatch)
-        }
-        // A route is renderable only after both its data and normal component
-        // chunk are ready. Its loader data is already available to descendants.
-        if (index >= retainedEnd) {
-          match.status = 'pending'
-        }
-      }
-    }
-    return result
-  })
 
-  const chunkOutcome = waitFor(
-    Promise.resolve().then(() => loadRouteChunk(route, undefined, onLazyReady)),
-    options[0 /* controller */].signal,
-  ).then(
-    () => undefined,
-    (cause): IndexedOutcome | undefined =>
-      lane[1 /* matches */].some(
-        (candidate, candidateIndex) =>
-          candidateIndex <= index &&
-          (candidate.status === 'error' ||
-            candidate.status === 'notFound' ||
-            candidate._notFound),
-      )
-        ? undefined
-        : [index, normalizeLaneError(router, lane, route, cause, options)],
+  // The async wrapper catches synchronous preload failures without deferring work.
+  const chunkOutcome = (async (): Promise<undefined> => {
+    const chunk = loadRouteChunk(route, undefined, onLazyReady)
+    if (chunk) {
+      await waitFor(chunk, options[0 /* controller */].signal)
+    }
+  })().catch((cause): IndexedOutcome | undefined =>
+    lane[1 /* matches */].some(
+      (candidate, candidateIndex) =>
+        candidateIndex <= index &&
+        (candidate.status === 'error' ||
+          candidate.status === 'notFound' ||
+          candidate._notFound),
+    )
+      ? undefined
+      : [index, normalizeLaneError(router, lane, route, cause, options)],
   )
   const chunkFailure = chunkOutcome.then((failure) =>
     outcome.then((result) => {

packages/router-core/tests/load-client-wait-for.test.ts

@@ -4,6 +4,72 @@ import { waitFor } from '../src/load-client'
 import { waitForRequest } from '../src/ssr/createRequestHandler'
 
 describe('waitFor', () => {
+  test.each(['resolve', 'reject'] as const)(
+    'removes its abort listener after the value %ss',
+    async (outcome) => {
+      const controller = new AbortController()
+      const add = vi.spyOn(controller.signal, 'addEventListener')
+      const remove = vi.spyOn(controller.signal, 'removeEventListener')
+      const value =
+        outcome === 'resolve' ? Promise.resolve(42) : Promise.reject(42)
+      const result = waitFor(value, controller.signal)
+      if (outcome === 'resolve') {
+        await expect(result).resolves.toBe(42)
+      } else {
+        await expect(result).rejects.toBe(42)
+      }
+      expect(add).toHaveBeenCalledOnce()
+      expect(remove).toHaveBeenCalledExactlyOnceWith(
+        'abort',
+        add.mock.calls[0]![1],
+      )
+    },
+  )
+
+  test('observes a late rejection and cleans up after an active wait is aborted', async () => {
+    const controller = new AbortController()
+    const add = vi.spyOn(controller.signal, 'addEventListener')
+    const remove = vi.spyOn(controller.signal, 'removeEventListener')
+    let rejectValue!: (error: Error) => void
+    const value = new Promise<never>((_, reject) => {
+      rejectValue = reject
+    })
+    const result = waitFor(value, controller.signal)
+    const canceled = expect(result).rejects.toBe(controller.signal)
+
+    controller.abort()
+    await canceled
+    rejectValue(new Error('late failure'))
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    expect(add).toHaveBeenCalledOnce()
+    expect(remove).toHaveBeenCalledExactlyOnceWith(
+      'abort',
+      add.mock.calls[0]![1],
+    )
+  })
+
+  test('removes its abort listener when reading a thenable throws', async () => {
+    const controller = new AbortController()
+    const add = vi.spyOn(controller.signal, 'addEventListener')
+    const remove = vi.spyOn(controller.signal, 'removeEventListener')
+    const error = new Error('then getter failed')
+    const value: PromiseLike<never> = {
+      get then(): never {
+        throw error
+      },
+    }
+
+    await expect(waitFor(value, controller.signal)).rejects.toBe(error)
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    expect(add).toHaveBeenCalledOnce()
+    expect(remove).toHaveBeenCalledExactlyOnceWith(
+      'abort',
+      add.mock.calls[0]![1],
+    )
+  })
+
   test('observes a rejected value when the signal is already aborted', async () => {
     const controller = new AbortController()
     const error = new Error('late failure')

packages/router-core/tests/ready-route-loading.test.ts

@@ -0,0 +1,136 @@
+import { expect, test, vi } from 'vitest'
+import { createMemoryHistory } from '@tanstack/history'
+import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src'
+import { createTestRouter } from './routerTestUtils'
+
+test.each(['resolve', 'reject', 'supersede'] as const)(
+  'a fresh cached loader still handles a new component chunk: %s',
+  async (outcome) => {
+    const data = { value: 'cached' }
+    const loader = vi.fn(() => data)
+    const root = new BaseRootRoute({})
+    const cached = new BaseRoute({
+      getParentRoute: () => root,
+      path: '/cached',
+      loader,
+      staleTime: Infinity,
+      gcTime: Infinity,
+    })
+    const other = new BaseRoute({
+      getParentRoute: () => root,
+      path: '/other',
+    })
+    const router = createTestRouter({
+      routeTree: root.addChildren([cached, other]),
+      history: createMemoryHistory({ initialEntries: ['/cached'] }),
+    })
+    await router.load()
+    await router.navigate({ to: '/other' })
+
+    const chunk = createControlledPromise<void>()
+    const started = createControlledPromise<void>()
+    cached.update({
+      component: Object.assign(() => null, {
+        preload: () => {
+          started.resolve()
+          return chunk
+        },
+      }),
+    })
+    let settled = false
+    const navigation = router.navigate({ to: '/cached' }).then(() => {
+      settled = true
+    })
+    await started
+    expect(settled).toBe(false)
+    expect(loader).toHaveBeenCalledOnce()
+
+    const error = new Error('Chunk failed')
+    if (outcome === 'supersede') {
+      await router.navigate({ to: '/other' })
+      await navigation
+      expect(router.state.location.pathname).toBe('/other')
+      chunk.reject(error)
+    } else {
+      if (outcome === 'resolve') {
+        chunk.resolve()
+      } else {
+        chunk.reject(error)
+      }
+      await navigation
+      const match = router.state.matches.find(
+        (candidate) => candidate.routeId === cached.id,
+      )!
+      expect(match.status).toBe(outcome === 'resolve' ? 'success' : 'error')
+      if (outcome === 'resolve') {
+        expect(match.loaderData).toBe(data)
+      } else {
+        expect(match.error).toBe(error)
+      }
+    }
+    expect(loader).toHaveBeenCalledOnce()
+  },
+)
+
+test('a synchronous component preload failure is caught and can be retried', async () => {
+  const error = new Error('Synchronous preload failure')
+  const preload = vi.fn(() => {
+    throw error
+  })
+  const root = new BaseRootRoute({})
+  const route = new BaseRoute({
+    getParentRoute: () => root,
+    path: '/route',
+    component: Object.assign(() => null, { preload }),
+  })
+  const router = createTestRouter({
+    routeTree: root.addChildren([route]),
+    history: createMemoryHistory({ initialEntries: ['/route'] }),
+  })
+
+  await router.load()
+  expect(router.state.matches.at(-1)).toMatchObject({
+    status: 'error',
+    error,
+  })
+
+  route.update({ component: () => null })
+  await router.load()
+  expect(router.state.matches.at(-1)?.status).toBe('success')
+  expect(preload).toHaveBeenCalledOnce()
+})
+
+test('a synchronous loader failure takes precedence over a synchronous component preload failure', async () => {
+  const loaderError = new Error('Loader failed')
+  const chunkError = new Error('Component preload failed')
+  const loader = vi.fn(() => {
+    throw loaderError
+  })
+  const preload = vi.fn(() => {
+    throw chunkError
+  })
+  const root = new BaseRootRoute({})
+  const route = new BaseRoute({
+    getParentRoute: () => root,
+    path: '/route',
+    loader,
+    component: Object.assign(() => null, { preload }),
+    errorComponent: () => null,
+  })
+  const history = createMemoryHistory({ initialEntries: ['/route'] })
+  const router = createTestRouter({
+    routeTree: root.addChildren([route]),
+    history,
+  })
+  try {
+    await router.load()
+    expect(loader).toHaveBeenCalledOnce()
+    expect(preload).toHaveBeenCalledOnce()
+    expect(router.state.matches.at(-1)).toMatchObject({
+      status: 'error',
+      error: loaderError,
+    })
+  } finally {
+    history.destroy()
+  }
+})