refactor(router-core): head, scripts, headers are executed in parallel and can skip store update #4925
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…l and can skip store update
|
View your CI Pipeline Execution ↗ for commit 4882c46
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
| styles, | ||
| } | ||
|
|
||
| return Promise.all([ |
There was a problem hiding this comment.
why no await Promise.all()?
personally, i like async/await syntax more than "then".
unless there is a technical reason?
There was a problem hiding this comment.
If we use await here, the function needs to be async, which means that it will always return a Promise, even if we returned early. I'd like to be able to do the if('then' in foo) thing where this function is called, but we can't do that if the function is async.
Sheraff
added a commit
that referenced
this pull request
Aug 15, 2025
) The `store-updates-during-navigation.test.tsx` file tests how many times `updateMatch` is called during a navigation (i.e. how many times the store is updated, and all subscribers re-run). In this PR we clean up this test file to allow us to more easily test various navigation types. We also add a test for "redirection inside beforeLoad" whose result will be improved by #4925 --------- 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>
This was referenced Aug 16, 2025
Merged
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 -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following #4916, this PR keeps reducing the number of store updates (
__store.setStatethroughupdateMatch).We can skip
route.options.head(),route.options.scripts(), androute.options.headers()calls, if they are not defined, and not even callupdateMatchafter.Additionally,
head,scriptsandheadersare 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:
cancelMatch, we were resetting the timeout id toundefinedbefore callingclearTimeout()with ithandleRedirectAndNotFoundis sometimes called w/matchbeingundefined(in case of a redirecting match during preload), which went undetected because of heavymatch!assertions, and the manytry/catchblocksWith this PR, the
redirection in preloadtest instore-updates-during-navigationgoes from 11 updates during the preload to 8 updates.Should be a partial improvement of #4359