refactor(router-core): Reduce navigation work by making some properties non-reactive (#4916)

Some of the properties of a route match in the store do not need to be
reactive (i.e. reactive = trigger the execution of all subscribers when
they change).

This PR proposes we add a `_nonReactive` key for storing these
properties.

Changes to properties in `_nonReactive` can be made like on a regular
object, without the need to call `__store.setState`, and without making
a shallow copy of the object.

The following properties were moved to `_nonReactive` (but more would be
welcome if we can find which ones):
- `beforeLoadPromise`
- `loaderPromise`
- `pendingTimeout`
- `loadPromise`
- `displayPendingPromise`
- `minPendingPromise`
- `dehydrated`

This results in the removal of 4 `updateMatch` calls in `router-core`.
At least 1 of which is in the hot-path of a navigation (`loadMatches` >
`validResolvedMatches.forEach`). And the removal of 2 `updateMatch`
calls in `packages/react-router/src/Match.tsx` (and solid-router too).

The `ssr` property was also updated to not be reactive, but kept outside
of `_nonReactive` because it is part of the public API. This removes
another call to `updateMatch`. This key is an exception, and if in the
future we find a need to have more "public but non-reactive" properties,
we could consider
- hiding internal props in a separate type, or
- adding a dev proxy to ensure non-reactive props aren't accessed inside
`select` functions

---

This PR adds a unit-test in `react-router` that counts how many times
the `select` inside a `useRouterState` hook is called during a
navigation. Without this PR, the result of this test is **26 calls**.
With this PR, we're down to **19 calls**.

---

Should be a partial improvement of
#4359

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

Author: Sheraff

docs/router/framework/react/api/router/RouteMatchType.md

@@ -18,7 +18,6 @@ interface RouteMatch {
   paramsError: unknown
   searchError: unknown
   updatedAt: number
-  loadPromise?: Promise<void>
   loaderData?: Route['loaderData']
   context: Route['allContext']
   search: Route['fullSearchSchema']

packages/react-router/src/Match.tsx

@@ -6,7 +6,6 @@ import {
   getLocationChangeInfo,
   isNotFound,
   isRedirect,
-  pick,
   rootRouteId,
 } from '@tanstack/router-core'
 import { CatchBoundary, ErrorComponent } from './CatchBoundary'
@@ -37,7 +36,11 @@ export const Match = React.memo(function MatchImpl({
         match,
         `Could not find match for matchId "${matchId}". Please file an issue!`,
       )
-      return pick(match, ['routeId', 'ssr', '_displayPending'])
+      return {
+        routeId: match.routeId,
+        ssr: match.ssr,
+        _displayPending: match._displayPending,
+      }
     },
     structuralSharing: true as any,
   })
@@ -204,13 +207,13 @@ export const MatchInner = React.memo(function MatchInnerImpl({
       return {
         key,
         routeId,
-        match: pick(match, [
-          'id',
-          'status',
-          'error',
-          '_forcePending',
-          '_displayPending',
-        ]),
+        match: {
+          id: match.id,
+          status: match.status,
+          error: match.error,
+          _forcePending: match._forcePending,
+          _displayPending: match._displayPending,
+        },
       }
     },
     structuralSharing: true as any,
@@ -227,43 +230,38 @@ export const MatchInner = React.memo(function MatchInnerImpl({
   }, [key, route.options.component, router.options.defaultComponent])
 
   if (match._displayPending) {
-    throw router.getMatch(match.id)?.displayPendingPromise
+    throw router.getMatch(match.id)?._nonReactive.displayPendingPromise
   }
 
   if (match._forcePending) {
-    throw router.getMatch(match.id)?.minPendingPromise
+    throw router.getMatch(match.id)?._nonReactive.minPendingPromise
   }
 
   // see also hydrate() in packages/router-core/src/ssr/ssr-client.ts
   if (match.status === 'pending') {
     // We're pending, and if we have a minPendingMs, we need to wait for it
     const pendingMinMs =
       route.options.pendingMinMs ?? router.options.defaultPendingMinMs
+    if (pendingMinMs) {
+      const routerMatch = router.getMatch(match.id)
+      if (routerMatch && !routerMatch._nonReactive.minPendingPromise) {
+        // Create a promise that will resolve after the minPendingMs
+        if (!router.isServer) {
+          const minPendingPromise = createControlledPromise<void>()
+
+          Promise.resolve().then(() => {
+            routerMatch._nonReactive.minPendingPromise = minPendingPromise
+          })
 
-    if (pendingMinMs && !router.getMatch(match.id)?.minPendingPromise) {
-      // Create a promise that will resolve after the minPendingMs
-      if (!router.isServer) {
-        const minPendingPromise = createControlledPromise<void>()
-
-        Promise.resolve().then(() => {
-          router.updateMatch(match.id, (prev) => ({
-            ...prev,
-            minPendingPromise,
-          }))
-        })
-
-        setTimeout(() => {
-          minPendingPromise.resolve()
-
-          // We've handled the minPendingPromise, so we can delete it
-          router.updateMatch(match.id, (prev) => ({
-            ...prev,
-            minPendingPromise: undefined,
-          }))
-        }, pendingMinMs)
+          setTimeout(() => {
+            minPendingPromise.resolve()
+            // We've handled the minPendingPromise, so we can delete it
+            routerMatch._nonReactive.minPendingPromise = undefined
+          }, pendingMinMs)
+        }
       }
     }
-    throw router.getMatch(match.id)?.loadPromise
+    throw router.getMatch(match.id)?._nonReactive.loadPromise
   }
 
   if (match.status === 'notFound') {
@@ -280,7 +278,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)?.loadPromise
+    throw router.getMatch(match.id)?._nonReactive.loadPromise
   }
 
   if (match.status === 'error') {

packages/react-router/tests/store-updates-during-navigation.test.tsx

@@ -0,0 +1,78 @@
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { act, cleanup, render, screen, waitFor } from '@testing-library/react'
+import {
+  Link,
+  Outlet,
+  RouterProvider,
+  createRootRoute,
+  createRoute,
+  createRouter,
+  useRouterState,
+} from '../src'
+import type { RouteComponent } from '../src'
+
+afterEach(() => {
+  window.history.replaceState(null, 'root', '/')
+  cleanup()
+})
+
+function setup({ RootComponent }: { RootComponent: RouteComponent }) {
+  const rootRoute = createRootRoute({
+    component: RootComponent,
+  })
+  const indexRoute = createRoute({
+    getParentRoute: () => rootRoute,
+    path: '/',
+    component: () => (
+      <>
+        <h1>IndexTitle</h1>
+        <Link to="/posts">Posts</Link>
+      </>
+    ),
+  })
+
+  const postsRoute = createRoute({
+    getParentRoute: () => rootRoute,
+    path: '/posts',
+    beforeLoad: () => new Promise<void>((resolve) => setTimeout(resolve, 100)),
+    loader: () => new Promise<void>((resolve) => setTimeout(resolve, 100)),
+    component: () => <h1>PostsTitle</h1>,
+  })
+
+  const router = createRouter({
+    routeTree: rootRoute.addChildren([indexRoute, postsRoute]),
+    defaultPendingMs: 100,
+    defaultPendingMinMs: 300,
+    defaultPendingComponent: () => <p>Loading...</p>,
+  })
+
+  return render(<RouterProvider router={router} />)
+}
+
+describe('Store updates during navigation', () => {
+  it("isn't called *too many* times", async () => {
+    const select = vi.fn()
+
+    setup({
+      RootComponent: () => {
+        useRouterState({ select })
+        return <Outlet />
+      },
+    })
+
+    // navigate to /posts
+    const link = await waitFor(() =>
+      screen.getByRole('link', { name: 'Posts' }),
+    )
+    const before = select.mock.calls.length
+    act(() => link.click())
+    const title = await waitFor(() => screen.getByText('PostsTitle'))
+    expect(title).toBeInTheDocument()
+    const after = select.mock.calls.length
+
+    // This number should be as small as possible to minimize the amount of work
+    // that needs to be done during a navigation.
+    // Any change that increases this number should be investigated.
+    expect(after - before).toBe(19)
+  })
+})

packages/router-core/src/Matches.ts

@@ -136,11 +136,18 @@ export interface RouteMatch<
   paramsError: unknown
   searchError: unknown
   updatedAt: number
-  loadPromise?: ControlledPromise<void>
-  /** @internal */
-  beforeLoadPromise?: ControlledPromise<void>
-  /** @internal */
-  loaderPromise?: ControlledPromise<void>
+  _nonReactive: {
+    /** @internal */
+    beforeLoadPromise?: ControlledPromise<void>
+    /** @internal */
+    loaderPromise?: ControlledPromise<void>
+    /** @internal */
+    pendingTimeout?: ReturnType<typeof setTimeout>
+    loadPromise?: ControlledPromise<void>
+    displayPendingPromise?: Promise<void>
+    minPendingPromise?: ControlledPromise<void>
+    dehydrated?: boolean
+  }
   loaderData?: TLoaderData
   /** @internal */
   __routeContext: Record<string, unknown>
@@ -158,12 +165,9 @@ export interface RouteMatch<
   headers?: Record<string, string>
   globalNotFound?: boolean
   staticData: StaticDataRouteOption
-  minPendingPromise?: ControlledPromise<void>
-  pendingTimeout?: ReturnType<typeof setTimeout>
+  /** This attribute is not reactive */
   ssr?: boolean | 'data-only'
-  _dehydrated?: boolean
   _forcePending?: boolean
-  displayPendingPromise?: Promise<void>
   _displayPending?: boolean
 }