refactor(router-core): Verify changes are necessary before updating store (#4964)

Following #4916,
#4925,
#4926, and
#4928, this PR keeps reducing the
number of store updates (`__store.setState` through `updateMatch`).

We check the values we want to set in the store are *actually different*
from what is already in the store before calling `updateMatch`

---

In the `store-updates-during-navigation` test tracking the number of
executions of a `useRouterState > select` method during a navigation,
our main test case goes from **14 calls** without this PR, to **10
calls** with this PR. Most test cases show significant improvements as
well.

---

Should be a partial improvement of
#4359

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Prevented loss of previously loaded data during navigation when a
loader yields no value.
* Stabilized preloading to avoid redundant updates and unnecessary state
churn.
* Improved loading-state cleanup after navigation, reducing flicker and
inconsistent “fetching” indicators.

* **Performance**
* Reduced unnecessary state updates and re-renders during and after
preloads/loads for smoother navigation.

* **Tests**
* Updated async navigation tests to reflect refined timing and
data-return behavior and added a helper to simulate delayed async
results.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: Sheraff

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

@@ -104,12 +104,15 @@ async function run({ select }: ReturnType<typeof setup>) {
   return after - before
 }
 
+function resolveAfter(ms: number, value: any) {
+  return new Promise<void>((resolve) => setTimeout(() => resolve(value), ms))
+}
+
 describe("Store doesn't update *too many* times during navigation", () => {
   test('async loader, async beforeLoad, pendingMs', async () => {
     const params = setup({
-      beforeLoad: () =>
-        new Promise<void>((resolve) => setTimeout(resolve, 100)),
-      loader: () => new Promise<void>((resolve) => setTimeout(resolve, 100)),
+      beforeLoad: () => resolveAfter(100, { foo: 'bar' }),
+      loader: () => resolveAfter(100, { hello: 'world' }),
       defaultPendingMs: 100,
       defaultPendingMinMs: 300,
     })
@@ -119,7 +122,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
     // 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(updates).toBe(14)
+    expect(updates).toBe(10)
   })
 
   test('redirection in preload', async () => {
@@ -137,13 +140,13 @@ describe("Store doesn't update *too many* times during navigation", () => {
     // 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(updates).toBe(6)
+    expect(updates).toBe(5)
   })
 
   test('sync beforeLoad', async () => {
     const params = setup({
       beforeLoad: () => ({ foo: 'bar' }),
-      loader: () => new Promise<void>((resolve) => setTimeout(resolve, 100)),
+      loader: () => resolveAfter(100, { hello: 'world' }),
       defaultPendingMs: 100,
       defaultPendingMinMs: 300,
     })
@@ -153,7 +156,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
     // 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(updates).toBe(13)
+    expect(updates).toBe(9)
   })
 
   test('nothing', async () => {
@@ -164,8 +167,8 @@ describe("Store doesn't update *too many* times during navigation", () => {
     // 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(updates).toBeGreaterThanOrEqual(10) // WARN: this is flaky, and sometimes (rarely) is 11
-    expect(updates).toBeLessThanOrEqual(11)
+    expect(updates).toBeGreaterThanOrEqual(6) // WARN: this is flaky, and sometimes (rarely) is 7
+    expect(updates).toBeLessThanOrEqual(7)
   })
 
   test('not found in beforeLoad', async () => {
@@ -205,6 +208,6 @@ describe("Store doesn't update *too many* times during navigation", () => {
     // 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(updates).toBe(19)
+    expect(updates).toBe(15)
   })
 })

packages/router-core/src/router.ts

@@ -2697,10 +2697,12 @@ export class RouterCore<
             this.getMatch(matchId),
             loaderData,
           )
-          innerLoadContext.updateMatch(matchId, (prev) => ({
-            ...prev,
-            loaderData,
-          }))
+          if (loaderData !== undefined) {
+            innerLoadContext.updateMatch(matchId, (prev) => ({
+              ...prev,
+              loaderData,
+            }))
+          }
         }
 
         // Lazy option can modify the route options,
@@ -2837,14 +2839,16 @@ export class RouterCore<
             )
           : shouldReloadOption
 
-      innerLoadContext.updateMatch(matchId, (prev) => {
-        prev._nonReactive.loaderPromise = createControlledPromise<void>()
-        return {
+      const nextPreload =
+        !!preload && !this.state.matches.some((d) => d.id === matchId)
+      const match = this.getMatch(matchId)!
+      match._nonReactive.loaderPromise = createControlledPromise<void>()
+      if (nextPreload !== match.preload) {
+        innerLoadContext.updateMatch(matchId, (prev) => ({
           ...prev,
-          preload:
-            !!preload && !this.state.matches.some((d) => d.id === matchId),
-        }
-      })
+          preload: nextPreload,
+        }))
+      }
 
       // If the route is successful and still fresh, just resolve
       const { status, invalid } = this.getMatch(matchId)!
@@ -2886,23 +2890,24 @@ export class RouterCore<
         }
       }
     }
+    const match = this.getMatch(matchId)!
     if (!loaderIsRunningAsync) {
-      const match = this.getMatch(matchId)!
       match._nonReactive.loaderPromise?.resolve()
       match._nonReactive.loadPromise?.resolve()
     }
 
-    innerLoadContext.updateMatch(matchId, (prev) => {
-      clearTimeout(prev._nonReactive.pendingTimeout)
-      prev._nonReactive.pendingTimeout = undefined
-      if (!loaderIsRunningAsync) prev._nonReactive.loaderPromise = undefined
-      prev._nonReactive.dehydrated = undefined
-      return {
+    clearTimeout(match._nonReactive.pendingTimeout)
+    match._nonReactive.pendingTimeout = undefined
+    if (!loaderIsRunningAsync) match._nonReactive.loaderPromise = undefined
+    match._nonReactive.dehydrated = undefined
+    const nextIsFetching = loaderIsRunningAsync ? match.isFetching : false
+    if (nextIsFetching !== match.isFetching || match.invalid !== false) {
+      innerLoadContext.updateMatch(matchId, (prev) => ({
         ...prev,
-        isFetching: loaderIsRunningAsync ? prev.isFetching : false,
+        isFetching: nextIsFetching,
         invalid: false,
-      }
-    })
+      }))
+    }
     return this.getMatch(matchId)!
   }