Skip to content

Commit fe3bb45

Browse files
Sheraffautofix-ci[bot]
Sheraff
and
autofix-ci[bot]
authored
refactor(router-core): skip loader and store update if nothing to await (#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>
1 parent 191ad1e commit fe3bb45

File tree

6 files changed

+93
-47
lines changed

6 files changed

+93
-47
lines changed

‎packages/react-router/src/Match.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({
248248
if (!router.isServer) {
249249
const minPendingPromise = createControlledPromise<void>()
250250

251-
Promise.resolve().then(() => {
252-
routerMatch._nonReactive.minPendingPromise = minPendingPromise
253-
})
251+
routerMatch._nonReactive.minPendingPromise = minPendingPromise
254252

255253
setTimeout(() => {
256254
minPendingPromise.resolve()

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
110110
// This number should be as small as possible to minimize the amount of work
111111
// that needs to be done during a navigation.
112112
// Any change that increases this number should be investigated.
113-
expect(updates).toBe(19)
113+
expect(updates).toBe(17)
114114
})
115115

116116
test('redirection in preload', async () => {

‎packages/router-core/src/route.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,10 @@ export interface Route<
589589
TBeforeLoadFn
590590
>
591591
isRoot: TParentRoute extends AnyRoute ? true : false
592-
_componentsPromise?: Promise<Array<void>>
592+
/** @internal */
593+
_componentsPromise?: Promise<void>
594+
/** @internal */
595+
_componentsLoaded?: boolean
593596
lazyFn?: () => Promise<
594597
LazyRoute<
595598
Route<
@@ -610,7 +613,10 @@ export interface Route<
610613
>
611614
>
612615
>
616+
/** @internal */
613617
_lazyPromise?: Promise<void>
618+
/** @internal */
619+
_lazyLoaded?: boolean
614620
rank: number
615621
to: TrimPathRight<TFullPath>
616622
init: (opts: { originalIndex: number }) => void
@@ -1406,8 +1412,10 @@ export class BaseRoute<
14061412
>
14071413
>
14081414
>
1415+
/** @internal */
14091416
_lazyPromise?: Promise<void>
1410-
_componentsPromise?: Promise<Array<void>>
1417+
/** @internal */
1418+
_componentsPromise?: Promise<void>
14111419

14121420
constructor(
14131421
options?: RouteOptions<

‎packages/router-core/src/router.ts

Lines changed: 70 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
createControlledPromise,
1010
deepEqual,
1111
functionalUpdate,
12+
isPromise,
1213
last,
1314
pick,
1415
replaceEqualDeep,
@@ -2508,11 +2509,9 @@ export class RouterCore<
25082509
})
25092510
}
25102511

2511-
const potentialPendingMinPromise = async () => {
2512+
const potentialPendingMinPromise = () => {
25122513
const latestMatch = this.getMatch(matchId)!
2513-
if (latestMatch._nonReactive.minPendingPromise) {
2514-
await latestMatch._nonReactive.minPendingPromise
2515-
}
2514+
return latestMatch._nonReactive.minPendingPromise
25162515
}
25172516

25182517
const prevMatch = this.getMatch(matchId)!
@@ -2621,41 +2620,63 @@ export class RouterCore<
26212620
try {
26222621
if (
26232622
!this.isServer ||
2624-
(this.isServer &&
2625-
this.getMatch(matchId)!.ssr === true)
2623+
this.getMatch(matchId)!.ssr === true
26262624
) {
26272625
this.loadRouteChunk(route)
26282626
}
26292627

2630-
updateMatch(matchId, (prev) => ({
2631-
...prev,
2632-
isFetching: 'loader',
2633-
}))
2634-
26352628
// Kick off the loader!
2636-
const loaderData =
2637-
await route.options.loader?.(getLoaderContext())
2638-
2639-
handleRedirectAndNotFound(
2640-
this.getMatch(matchId),
2641-
loaderData,
2629+
const loaderResult =
2630+
route.options.loader?.(getLoaderContext())
2631+
const loaderResultIsPromise =
2632+
route.options.loader && isPromise(loaderResult)
2633+
2634+
const willLoadSomething = !!(
2635+
loaderResultIsPromise ||
2636+
route._lazyPromise ||
2637+
route._componentsPromise ||
2638+
route.options.head ||
2639+
route.options.scripts ||
2640+
route.options.headers ||
2641+
this.getMatch(matchId)!._nonReactive
2642+
.minPendingPromise
26422643
)
2643-
updateMatch(matchId, (prev) => ({
2644-
...prev,
2645-
loaderData,
2646-
}))
2644+
2645+
if (willLoadSomething) {
2646+
updateMatch(matchId, (prev) => ({
2647+
...prev,
2648+
isFetching: 'loader',
2649+
}))
2650+
}
2651+
2652+
if (route.options.loader) {
2653+
const loaderData = loaderResultIsPromise
2654+
? await loaderResult
2655+
: loaderResult
2656+
2657+
handleRedirectAndNotFound(
2658+
this.getMatch(matchId),
2659+
loaderData,
2660+
)
2661+
updateMatch(matchId, (prev) => ({
2662+
...prev,
2663+
loaderData,
2664+
}))
2665+
}
26472666

26482667
// Lazy option can modify the route options,
26492668
// so we need to wait for it to resolve before
26502669
// we can use the options
2651-
await route._lazyPromise
2670+
if (route._lazyPromise) await route._lazyPromise
26522671
const headResult = executeHead()
26532672
const head = headResult ? await headResult : undefined
2654-
await potentialPendingMinPromise()
2673+
const pendingPromise = potentialPendingMinPromise()
2674+
if (pendingPromise) await pendingPromise
26552675

26562676
// Last but not least, wait for the the components
26572677
// to be preloaded before we resolve the match
2658-
await route._componentsPromise
2678+
if (route._componentsPromise)
2679+
await route._componentsPromise
26592680
updateMatch(matchId, (prev) => ({
26602681
...prev,
26612682
error: undefined,
@@ -2890,33 +2911,44 @@ export class RouterCore<
28902911
}
28912912

28922913
loadRouteChunk = (route: AnyRoute) => {
2893-
if (route._lazyPromise === undefined) {
2914+
if (!route._lazyLoaded && route._lazyPromise === undefined) {
28942915
if (route.lazyFn) {
28952916
route._lazyPromise = route.lazyFn().then((lazyRoute) => {
28962917
// explicitly don't copy over the lazy route's id
28972918
const { id: _id, ...options } = lazyRoute.options
28982919
Object.assign(route.options, options)
2920+
route._lazyLoaded = true
2921+
route._lazyPromise = undefined // gc promise, we won't need it anymore
28992922
})
29002923
} else {
2901-
route._lazyPromise = Promise.resolve()
2924+
route._lazyLoaded = true
29022925
}
29032926
}
29042927

29052928
// If for some reason lazy resolves more lazy components...
2906-
// We'll wait for that before pre attempt to preload any
2929+
// We'll wait for that before we attempt to preload the
29072930
// components themselves.
2908-
if (route._componentsPromise === undefined) {
2909-
route._componentsPromise = route._lazyPromise.then(() =>
2910-
Promise.all(
2911-
componentTypes.map(async (type) => {
2912-
const component = route.options[type]
2913-
if ((component as any)?.preload) {
2914-
await (component as any).preload()
2915-
}
2916-
}),
2917-
),
2918-
)
2931+
if (!route._componentsLoaded && route._componentsPromise === undefined) {
2932+
const loadComponents = () => {
2933+
const preloads = []
2934+
for (const type of componentTypes) {
2935+
const preload = (route.options[type] as any)?.preload
2936+
if (preload) preloads.push(preload())
2937+
}
2938+
if (preloads.length)
2939+
return Promise.all(preloads).then(() => {
2940+
route._componentsLoaded = true
2941+
route._componentsPromise = undefined // gc promise, we won't need it anymore
2942+
})
2943+
route._componentsLoaded = true
2944+
route._componentsPromise = undefined // gc promise, we won't need it anymore
2945+
return
2946+
}
2947+
route._componentsPromise = route._lazyPromise
2948+
? route._lazyPromise.then(loadComponents)
2949+
: loadComponents()
29192950
}
2951+
29202952
return route._componentsPromise
29212953
}
29222954

‎packages/router-core/src/utils.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,13 @@ export function isModuleNotFoundError(error: any): boolean {
473473
error.message.startsWith('Importing a module script failed')
474474
)
475475
}
476+
477+
export function isPromise<T>(
478+
value: Promise<Awaited<T>> | T,
479+
): value is Promise<Awaited<T>> {
480+
return Boolean(
481+
value &&
482+
typeof value === 'object' &&
483+
typeof (value as Promise<T>).then === 'function',
484+
)
485+
}

‎packages/solid-router/src/Match.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,7 @@ export const MatchInner = (props: { matchId: string }): any => {
263263
if (!router.isServer) {
264264
const minPendingPromise = createControlledPromise<void>()
265265

266-
Promise.resolve().then(() => {
267-
routerMatch._nonReactive.minPendingPromise = minPendingPromise
268-
})
266+
routerMatch._nonReactive.minPendingPromise = minPendingPromise
269267

270268
setTimeout(() => {
271269
minPendingPromise.resolve()

0 commit comments

Comments
 (0)