Skip to content

Conversation

Sheraff
Copy link
Contributor

@Sheraff Sheraff commented Aug 11, 2025

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

Copy link

nx-cloud bot commented Aug 11, 2025

View your CI Pipeline Execution ↗ for commit fbdc46a

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 1m 47s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 3s View ↗

☁️ Nx Cloud last updated this comment at 2025-08-11 11:37:38 UTC

Copy link

pkg-pr-new bot commented Aug 11, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@4916

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@4916

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@4916

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@4916

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@4916

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@4916

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@4916

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@4916

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@4916

@tanstack/react-start-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-plugin@4916

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@4916

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@4916

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@4916

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@4916

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@4916

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@4916

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@4916

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@4916

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@4916

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@4916

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@4916

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@4916

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@4916

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@4916

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@4916

@tanstack/solid-start-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-plugin@4916

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@4916

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@4916

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@4916

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@4916

@tanstack/start-server-functions-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-client@4916

@tanstack/start-server-functions-fetcher

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-fetcher@4916

@tanstack/start-server-functions-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-server@4916

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@4916

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@4916

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@4916

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@4916

commit: fbdc46a

@github-actions github-actions bot added the documentation Everything documentation related label Aug 11, 2025
@Sheraff Sheraff self-assigned this Aug 11, 2025
@Sheraff Sheraff merged commit 916253a into main Aug 11, 2025
5 checks passed
@Sheraff Sheraff deleted the refactor-router-core-non-reactive-store-properties branch August 11, 2025 11:59
Sheraff added a commit that referenced this pull request Aug 15, 2025
…l and can skip store update (#4925)

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

We can skip `route.options.head()`, `route.options.scripts()`, and
`route.options.headers()` calls, if they are not defined, and not even
call `updateMatch` after.

Additionally, `head`, `scripts` and `headers` are now called in
parallel. They don't depend on one another, and so there is no reason to
delay their execution by calling them serially.

---

This PR also fixes 2 issues:
- in `cancelMatch`, we were resetting the timeout id to `undefined`
**before** calling `clearTimeout()` with it
- `handleRedirectAndNotFound` is sometimes called w/ `match` being
`undefined` (in case of a redirecting match during preload), which went
undetected because of heavy `match!` assertions, and the many
`try/catch` blocks

---

With this PR, the `redirection in preload` test in
`store-updates-during-navigation` goes from **11 updates** during the
preload to **8 updates**.

---

Should be a partial improvement of
#4359

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sheraff added a commit that referenced this pull request Aug 15, 2025
…it (#4926)

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

Reduce amount of work in `runLoader`:
- don't `updateMatch` to set `isFetching: 'loader'` if we know the
entire function will be synchronous
- don't await `route.options.loader()` if it is synchronous
- don't update store with `loaderData` if `route.options.loader` is not
defined
- don't `await route._lazyPromise` if it has already resolved
- don't `await route._componentsPromise` if it has already resolved
- don't `await minPendingPromise` if it is not defined or has already
resolved

For a sync `loader`, with already loaded route chunks, `runLoader`
should be synchronous and trigger a single `updateMatch` call.

---

The `store-updates-during-navigation` test tracking the number of
executions of a `useRouterState > select` method during a navigation
goes from **19 calls** without this PR, to **17 calls** with this PR.

---

Should be a partial improvement of
#4359

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sheraff added a commit that referenced this pull request Aug 16, 2025
…ptions is not defined (#4928)

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

Early bail out of route before load step if `route.options.beforeLoad`
is not defined

---

The `store-updates-during-navigation` test tracking the number of
executions of a `useRouterState > select` method during a navigation
goes from **17 calls** without this PR, to **14 calls** with this PR (or
even 13 calls if `beforeLoad` is synchronous).

---

Should be a partial improvement of
#4359

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

* **New Features**
* Added a not-found helper and router options to configure a default Not
Found component and a default preload strategy.

* **Refactor**
* Streamlined the before-load lifecycle for more deterministic
pending-state updates, earlier pending-timeout readiness, clearer
parameter/search error surfacing, and adjusted context composition;
loader error timing is more conservative.

* **Tests**
* Expanded test coverage with new scenarios (sync before-load, not-found
flow, hover-preload/navigation) and updated navigation update
expectations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sheraff added a commit that referenced this pull request Aug 16, 2025
…tore (#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 -->
tannerlinsley pushed a commit that referenced this pull request Aug 26, 2025
…l and can skip store update (#4925)

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

We can skip `route.options.head()`, `route.options.scripts()`, and
`route.options.headers()` calls, if they are not defined, and not even
call `updateMatch` after.

Additionally, `head`, `scripts` and `headers` are now called in
parallel. They don't depend on one another, and so there is no reason to
delay their execution by calling them serially.

---

This PR also fixes 2 issues:
- in `cancelMatch`, we were resetting the timeout id to `undefined`
**before** calling `clearTimeout()` with it
- `handleRedirectAndNotFound` is sometimes called w/ `match` being
`undefined` (in case of a redirecting match during preload), which went
undetected because of heavy `match!` assertions, and the many
`try/catch` blocks

---

With this PR, the `redirection in preload` test in
`store-updates-during-navigation` goes from **11 updates** during the
preload to **8 updates**.

---

Should be a partial improvement of
#4359

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
tannerlinsley pushed a commit that referenced this pull request Aug 26, 2025
…it (#4926)

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

Reduce amount of work in `runLoader`:
- don't `updateMatch` to set `isFetching: 'loader'` if we know the
entire function will be synchronous
- don't await `route.options.loader()` if it is synchronous
- don't update store with `loaderData` if `route.options.loader` is not
defined
- don't `await route._lazyPromise` if it has already resolved
- don't `await route._componentsPromise` if it has already resolved
- don't `await minPendingPromise` if it is not defined or has already
resolved

For a sync `loader`, with already loaded route chunks, `runLoader`
should be synchronous and trigger a single `updateMatch` call.

---

The `store-updates-during-navigation` test tracking the number of
executions of a `useRouterState > select` method during a navigation
goes from **19 calls** without this PR, to **17 calls** with this PR.

---

Should be a partial improvement of
#4359

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
tannerlinsley pushed a commit that referenced this pull request Aug 26, 2025
…ptions is not defined (#4928)

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

Early bail out of route before load step if `route.options.beforeLoad`
is not defined

---

The `store-updates-during-navigation` test tracking the number of
executions of a `useRouterState > select` method during a navigation
goes from **17 calls** without this PR, to **14 calls** with this PR (or
even 13 calls if `beforeLoad` is synchronous).

---

Should be a partial improvement of
#4359

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

* **New Features**
* Added a not-found helper and router options to configure a default Not
Found component and a default preload strategy.

* **Refactor**
* Streamlined the before-load lifecycle for more deterministic
pending-state updates, earlier pending-timeout readiness, clearer
parameter/search error surfacing, and adjusted context composition;
loader error timing is more conservative.

* **Tests**
* Expanded test coverage with new scenarios (sync before-load, not-found
flow, hover-preload/navigation) and updated navigation update
expectations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
tannerlinsley pushed a commit that referenced this pull request Aug 26, 2025
…tore (#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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant