Skip to content

refactor(router-core): skip loader and store update if nothing to await #4926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/react-router/src/Match.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({
if (!router.isServer) {
const minPendingPromise = createControlledPromise<void>()

Promise.resolve().then(() => {
routerMatch._nonReactive.minPendingPromise = minPendingPromise
})
routerMatch._nonReactive.minPendingPromise = minPendingPromise

setTimeout(() => {
minPendingPromise.resolve()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,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(19)
expect(updates).toBe(17)
})

test('redirection in preload', async () => {
Expand Down
12 changes: 10 additions & 2 deletions packages/router-core/src/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,10 @@ export interface Route<
TBeforeLoadFn
>
isRoot: TParentRoute extends AnyRoute ? true : false
_componentsPromise?: Promise<Array<void>>
/** @internal */
_componentsPromise?: Promise<void>
/** @internal */
_componentsLoaded?: boolean
lazyFn?: () => Promise<
LazyRoute<
Route<
Expand All @@ -610,7 +613,10 @@ export interface Route<
>
>
>
/** @internal */
_lazyPromise?: Promise<void>
/** @internal */
_lazyLoaded?: boolean
rank: number
to: TrimPathRight<TFullPath>
init: (opts: { originalIndex: number }) => void
Expand Down Expand Up @@ -1406,8 +1412,10 @@ export class BaseRoute<
>
>
>
/** @internal */
_lazyPromise?: Promise<void>
_componentsPromise?: Promise<Array<void>>
/** @internal */
_componentsPromise?: Promise<void>

constructor(
options?: RouteOptions<
Expand Down
108 changes: 70 additions & 38 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
createControlledPromise,
deepEqual,
functionalUpdate,
isPromise,
last,
pick,
replaceEqualDeep,
Expand Down Expand Up @@ -2508,11 +2509,9 @@ export class RouterCore<
})
}

const potentialPendingMinPromise = async () => {
const potentialPendingMinPromise = () => {
const latestMatch = this.getMatch(matchId)!
if (latestMatch._nonReactive.minPendingPromise) {
await latestMatch._nonReactive.minPendingPromise
}
return latestMatch._nonReactive.minPendingPromise
}

const prevMatch = this.getMatch(matchId)!
Expand Down Expand Up @@ -2621,41 +2620,63 @@ export class RouterCore<
try {
if (
!this.isServer ||
(this.isServer &&
this.getMatch(matchId)!.ssr === true)
this.getMatch(matchId)!.ssr === true
) {
this.loadRouteChunk(route)
}

updateMatch(matchId, (prev) => ({
...prev,
isFetching: 'loader',
}))

// Kick off the loader!
const loaderData =
await route.options.loader?.(getLoaderContext())

handleRedirectAndNotFound(
this.getMatch(matchId),
loaderData,
const loaderResult =
route.options.loader?.(getLoaderContext())
const loaderResultIsPromise =
route.options.loader && isPromise(loaderResult)

const willLoadSomething = !!(
loaderResultIsPromise ||
route._lazyPromise ||
route._componentsPromise ||
route.options.head ||
route.options.scripts ||
route.options.headers ||
this.getMatch(matchId)!._nonReactive
.minPendingPromise
)
updateMatch(matchId, (prev) => ({
...prev,
loaderData,
}))

if (willLoadSomething) {
updateMatch(matchId, (prev) => ({
...prev,
isFetching: 'loader',
}))
}

if (route.options.loader) {
const loaderData = loaderResultIsPromise
? await loaderResult
: loaderResult

handleRedirectAndNotFound(
this.getMatch(matchId),
loaderData,
)
updateMatch(matchId, (prev) => ({
...prev,
loaderData,
}))
}

// Lazy option can modify the route options,
// so we need to wait for it to resolve before
// we can use the options
await route._lazyPromise
if (route._lazyPromise) await route._lazyPromise
const headResult = executeHead()
const head = headResult ? await headResult : undefined
await potentialPendingMinPromise()
const pendingPromise = potentialPendingMinPromise()
if (pendingPromise) await pendingPromise

// Last but not least, wait for the the components
// to be preloaded before we resolve the match
await route._componentsPromise
if (route._componentsPromise)
await route._componentsPromise
updateMatch(matchId, (prev) => ({
...prev,
error: undefined,
Expand Down Expand Up @@ -2890,33 +2911,44 @@ export class RouterCore<
}

loadRouteChunk = (route: AnyRoute) => {
if (route._lazyPromise === undefined) {
if (!route._lazyLoaded && route._lazyPromise === undefined) {
if (route.lazyFn) {
route._lazyPromise = route.lazyFn().then((lazyRoute) => {
// explicitly don't copy over the lazy route's id
const { id: _id, ...options } = lazyRoute.options
Object.assign(route.options, options)
route._lazyLoaded = true
route._lazyPromise = undefined // gc promise, we won't need it anymore
})
} else {
route._lazyPromise = Promise.resolve()
route._lazyLoaded = true
}
}

// If for some reason lazy resolves more lazy components...
// We'll wait for that before pre attempt to preload any
// We'll wait for that before we attempt to preload the
// components themselves.
if (route._componentsPromise === undefined) {
route._componentsPromise = route._lazyPromise.then(() =>
Promise.all(
componentTypes.map(async (type) => {
const component = route.options[type]
if ((component as any)?.preload) {
await (component as any).preload()
}
}),
),
)
if (!route._componentsLoaded && route._componentsPromise === undefined) {
const loadComponents = () => {
const preloads = []
for (const type of componentTypes) {
const preload = (route.options[type] as any)?.preload
if (preload) preloads.push(preload())
}
if (preloads.length)
return Promise.all(preloads).then(() => {
route._componentsLoaded = true
route._componentsPromise = undefined // gc promise, we won't need it anymore
})
route._componentsLoaded = true
route._componentsPromise = undefined // gc promise, we won't need it anymore
return
}
route._componentsPromise = route._lazyPromise
? route._lazyPromise.then(loadComponents)
: loadComponents()
}

return route._componentsPromise
}

Expand Down
10 changes: 10 additions & 0 deletions packages/router-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,3 +473,13 @@ export function isModuleNotFoundError(error: any): boolean {
error.message.startsWith('Importing a module script failed')
)
}

export function isPromise<T>(
value: Promise<Awaited<T>> | T,
): value is Promise<Awaited<T>> {
return Boolean(
value &&
typeof value === 'object' &&
typeof (value as Promise<T>).then === 'function',
)
}
4 changes: 1 addition & 3 deletions packages/solid-router/src/Match.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,7 @@ export const MatchInner = (props: { matchId: string }): any => {
if (!router.isServer) {
const minPendingPromise = createControlledPromise<void>()

Promise.resolve().then(() => {
routerMatch._nonReactive.minPendingPromise = minPendingPromise
})
routerMatch._nonReactive.minPendingPromise = minPendingPromise

setTimeout(() => {
minPendingPromise.resolve()
Expand Down