fix: clear pendingTimeout when match resolves (#4579)

Author: schiller-manuel

packages/react-router/tests/loaders.test.tsx

@@ -10,6 +10,7 @@ import {
 import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
 
 import { z } from 'zod'
+import { useEffect } from 'react'
 import {
   Link,
   Outlet,
@@ -645,3 +646,86 @@ test('reproducer #4546', async () => {
     expect(loaderData).toHaveTextContent('5')
   }
 })
+
+test('clears pendingTimeout when match resolves', async () => {
+  const defaultPendingComponentOnMountMock = vi.fn()
+  const nestedPendingComponentOnMountMock = vi.fn()
+  const fooPendingComponentOnMountMock = vi.fn()
+
+  function getPendingComponent(onMount: () => void) {
+    const PendingComponent = () => {
+      useEffect(() => {
+        onMount()
+      }, [])
+
+      return <div>Pending...</div>
+    }
+    return PendingComponent
+  }
+
+  const rootRoute = createRootRoute({})
+  const indexRoute = createRoute({
+    getParentRoute: () => rootRoute,
+    path: '/',
+    component: () => {
+      return (
+        <div>
+          <h1>Index page</h1>
+          <Link data-testid="link-to-foo" to="/nested/foo">
+            link to foo
+          </Link>
+        </div>
+      )
+    },
+  })
+  const nestedRoute = createRoute({
+    getParentRoute: () => rootRoute,
+    path: '/nested',
+    // this route does not specify pendingMinMs, so it will use the defaultPendingMs from the router
+    // which is set to WAIT_TIME * 2
+    // since the loader immediately resolves, the pending component must NOT be shown
+    pendingComponent: getPendingComponent(nestedPendingComponentOnMountMock),
+    loader: () => {
+      return 'nested'
+    },
+  })
+  const fooRoute = createRoute({
+    getParentRoute: () => nestedRoute,
+    path: '/foo',
+    // this route's loader takes WAIT_TIME * 5, so it will take longer than the defaultPendingMs
+    // however, this route specifies pendingMs as WAIT_TIME * 10,
+    // so this route's pending component must also NOT be shown
+    pendingComponent: getPendingComponent(fooPendingComponentOnMountMock),
+    pendingMs: WAIT_TIME * 10,
+    loader: async () => {
+      await sleep(WAIT_TIME * 5)
+    },
+    component: () => <div>Nested Foo page</div>,
+  })
+  const routeTree = rootRoute.addChildren([
+    nestedRoute.addChildren([fooRoute]),
+    indexRoute,
+  ])
+  const router = createRouter({
+    routeTree,
+    history,
+    defaultPendingMs: WAIT_TIME * 2,
+    defaultPendingComponent: getPendingComponent(
+      defaultPendingComponentOnMountMock,
+    ),
+  })
+
+  render(<RouterProvider router={router} />)
+  const linkToFoo = await screen.findByTestId('link-to-foo')
+  fireEvent.click(linkToFoo)
+  await router.latestLoadPromise
+  const fooElement = await screen.findByText('Nested Foo page')
+  expect(fooElement).toBeInTheDocument()
+
+  expect(router.state.location.href).toBe('/nested/foo')
+
+  // none of the pending components should have been called
+  expect(defaultPendingComponentOnMountMock).not.toHaveBeenCalled()
+  expect(nestedPendingComponentOnMountMock).not.toHaveBeenCalled()
+  expect(fooPendingComponentOnMountMock).not.toHaveBeenCalled()
+})

packages/router-core/src/router.ts

@@ -1391,7 +1391,13 @@ export class RouterCore<
     if (!match) return
 
     match.abortController.abort()
-    clearTimeout(match.pendingTimeout)
+    this.updateMatch(id, (prev) => {
+      clearTimeout(prev.pendingTimeout)
+      return {
+        ...prev,
+        pendingTimeout: undefined,
+      }
+    })
   }
 
   cancelMatches = () => {
@@ -2312,21 +2318,31 @@ export class RouterCore<
               )
 
               let executeBeforeLoad = true
-              if (
-                // 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)
-                existingMatch.beforeLoadPromise ||
-                existingMatch.loaderPromise
-              ) {
-                if (shouldPending) {
-                  setTimeout(() => {
+              const setupPendingTimeout = () => {
+                if (
+                  shouldPending &&
+                  this.getMatch(matchId)!.pendingTimeout === undefined
+                ) {
+                  const pendingTimeout = setTimeout(() => {
                     try {
                       // Update the match and prematurely resolve the loadMatches promise so that
                       // the pending component can start rendering
                       triggerOnReady()
                     } catch {}
                   }, pendingMs)
+                  updateMatch(matchId, (prev) => ({
+                    ...prev,
+                    pendingTimeout,
+                  }))
                 }
+              }
+              if (
+                // 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)
+                existingMatch.beforeLoadPromise ||
+                existingMatch.loaderPromise
+              ) {
+                setupPendingTimeout()
 
                 // Wait for the beforeLoad to resolve before we continue
                 await existingMatch.beforeLoadPromise
@@ -2354,21 +2370,6 @@ export class RouterCore<
                       beforeLoadPromise: createControlledPromise<void>(),
                     }
                   })
-                  const abortController = new AbortController()
-
-                  let pendingTimeout: ReturnType<typeof setTimeout>
-
-                  if (shouldPending) {
-                    // If we might show a pending component, we need to wait for the
-                    // pending promise to resolve before we start showing that state
-                    pendingTimeout = setTimeout(() => {
-                      try {
-                        // Update the match and prematurely resolve the loadMatches promise so that
-                        // the pending component can start rendering
-                        triggerOnReady()
-                      } catch {}
-                    }, pendingMs)
-                  }
 
                   const { paramsError, searchError } = this.getMatch(matchId)!
 
@@ -2380,6 +2381,10 @@ export class RouterCore<
                     handleSerialError(index, searchError, 'VALIDATE_SEARCH')
                   }
 
+                  setupPendingTimeout()
+
+                  const abortController = new AbortController()
+
                   const parentMatchContext =
                     parentMatch?.context ?? this.options.context ?? {}
 
@@ -2388,7 +2393,6 @@ export class RouterCore<
                     isFetching: 'beforeLoad',
                     fetchCount: prev.fetchCount + 1,
                     abortController,
-                    pendingTimeout,
                     context: {
                       ...parentMatchContext,
                       ...prev.__routeContext,
@@ -2758,16 +2762,22 @@ export class RouterCore<
                     loadPromise?.resolve()
                   }
 
-                  updateMatch(matchId, (prev) => ({
-                    ...prev,
-                    isFetching: loaderIsRunningAsync ? prev.isFetching : false,
-                    loaderPromise: loaderIsRunningAsync
-                      ? prev.loaderPromise
-                      : undefined,
-                    invalid: false,
-                    _dehydrated: undefined,
-                    _forcePending: undefined,
-                  }))
+                  updateMatch(matchId, (prev) => {
+                    clearTimeout(prev.pendingTimeout)
+                    return {
+                      ...prev,
+                      isFetching: loaderIsRunningAsync
+                        ? prev.isFetching
+                        : false,
+                      loaderPromise: loaderIsRunningAsync
+                        ? prev.loaderPromise
+                        : undefined,
+                      invalid: false,
+                      pendingTimeout: undefined,
+                      _dehydrated: undefined,
+                      _forcePending: undefined,
+                    }
+                  })
                   return this.getMatch(matchId)!
                 })(),
               )