Skip to content

Commit 544f62a

Browse files
Sheraffautofix-ci[bot]
authored and
tannerlinsley
committed
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 4e1725b commit 544f62a

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,
@@ -2501,11 +2502,9 @@ export class RouterCore<
25012502
})
25022503
}
25032504

2504-
const potentialPendingMinPromise = async () => {
2505+
const potentialPendingMinPromise = () => {
25052506
const latestMatch = this.getMatch(matchId)!
2506-
if (latestMatch._nonReactive.minPendingPromise) {
2507-
await latestMatch._nonReactive.minPendingPromise
2508-
}
2507+
return latestMatch._nonReactive.minPendingPromise
25092508
}
25102509

25112510
const prevMatch = this.getMatch(matchId)!
@@ -2614,41 +2613,63 @@ export class RouterCore<
26142613
try {
26152614
if (
26162615
!this.isServer ||
2617-
(this.isServer &&
2618-
this.getMatch(matchId)!.ssr === true)
2616+
this.getMatch(matchId)!.ssr === true
26192617
) {
26202618
this.loadRouteChunk(route)
26212619
}
26222620

2623-
updateMatch(matchId, (prev) => ({
2624-
...prev,
2625-
isFetching: 'loader',
2626-
}))
2627-
26282621
// Kick off the loader!
2629-
const loaderData =
2630-
await route.options.loader?.(getLoaderContext())
2631-
2632-
handleRedirectAndNotFound(
2633-
this.getMatch(matchId),
2634-
loaderData,
2622+
const loaderResult =
2623+
route.options.loader?.(getLoaderContext())
2624+
const loaderResultIsPromise =
2625+
route.options.loader && isPromise(loaderResult)
2626+
2627+
const willLoadSomething = !!(
2628+
loaderResultIsPromise ||
2629+
route._lazyPromise ||
2630+
route._componentsPromise ||
2631+
route.options.head ||
2632+
route.options.scripts ||
2633+
route.options.headers ||
2634+
this.getMatch(matchId)!._nonReactive
2635+
.minPendingPromise
26352636
)
2636-
updateMatch(matchId, (prev) => ({
2637-
...prev,
2638-
loaderData,
2639-
}))
2637+
2638+
if (willLoadSomething) {
2639+
updateMatch(matchId, (prev) => ({
2640+
...prev,
2641+
isFetching: 'loader',
2642+
}))
2643+
}
2644+
2645+
if (route.options.loader) {
2646+
const loaderData = loaderResultIsPromise
2647+
? await loaderResult
2648+
: loaderResult
2649+
2650+
handleRedirectAndNotFound(
2651+
this.getMatch(matchId),
2652+
loaderData,
2653+
)
2654+
updateMatch(matchId, (prev) => ({
2655+
...prev,
2656+
loaderData,
2657+
}))
2658+
}
26402659

26412660
// Lazy option can modify the route options,
26422661
// so we need to wait for it to resolve before
26432662
// we can use the options
2644-
await route._lazyPromise
2663+
if (route._lazyPromise) await route._lazyPromise
26452664
const headResult = executeHead()
26462665
const head = headResult ? await headResult : undefined
2647-
await potentialPendingMinPromise()
2666+
const pendingPromise = potentialPendingMinPromise()
2667+
if (pendingPromise) await pendingPromise
26482668

26492669
// Last but not least, wait for the the components
26502670
// to be preloaded before we resolve the match
2651-
await route._componentsPromise
2671+
if (route._componentsPromise)
2672+
await route._componentsPromise
26522673
updateMatch(matchId, (prev) => ({
26532674
...prev,
26542675
error: undefined,
@@ -2883,33 +2904,44 @@ export class RouterCore<
28832904
}
28842905

28852906
loadRouteChunk = (route: AnyRoute) => {
2886-
if (route._lazyPromise === undefined) {
2907+
if (!route._lazyLoaded && route._lazyPromise === undefined) {
28872908
if (route.lazyFn) {
28882909
route._lazyPromise = route.lazyFn().then((lazyRoute) => {
28892910
// explicitly don't copy over the lazy route's id
28902911
const { id: _id, ...options } = lazyRoute.options
28912912
Object.assign(route.options, options)
2913+
route._lazyLoaded = true
2914+
route._lazyPromise = undefined // gc promise, we won't need it anymore
28922915
})
28932916
} else {
2894-
route._lazyPromise = Promise.resolve()
2917+
route._lazyLoaded = true
28952918
}
28962919
}
28972920

28982921
// If for some reason lazy resolves more lazy components...
2899-
// We'll wait for that before pre attempt to preload any
2922+
// We'll wait for that before we attempt to preload the
29002923
// components themselves.
2901-
if (route._componentsPromise === undefined) {
2902-
route._componentsPromise = route._lazyPromise.then(() =>
2903-
Promise.all(
2904-
componentTypes.map(async (type) => {
2905-
const component = route.options[type]
2906-
if ((component as any)?.preload) {
2907-
await (component as any).preload()
2908-
}
2909-
}),
2910-
),
2911-
)
2924+
if (!route._componentsLoaded && route._componentsPromise === undefined) {
2925+
const loadComponents = () => {
2926+
const preloads = []
2927+
for (const type of componentTypes) {
2928+
const preload = (route.options[type] as any)?.preload
2929+
if (preload) preloads.push(preload())
2930+
}
2931+
if (preloads.length)
2932+
return Promise.all(preloads).then(() => {
2933+
route._componentsLoaded = true
2934+
route._componentsPromise = undefined // gc promise, we won't need it anymore
2935+
})
2936+
route._componentsLoaded = true
2937+
route._componentsPromise = undefined // gc promise, we won't need it anymore
2938+
return
2939+
}
2940+
route._componentsPromise = route._lazyPromise
2941+
? route._lazyPromise.then(loadComponents)
2942+
: loadComponents()
29122943
}
2944+
29132945
return route._componentsPromise
29142946
}
29152947

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,3 +432,13 @@ export function isModuleNotFoundError(error: any): boolean {
432432
error.message.startsWith('Importing a module script failed')
433433
)
434434
}
435+
436+
export function isPromise<T>(
437+
value: Promise<Awaited<T>> | T,
438+
): value is Promise<Awaited<T>> {
439+
return Boolean(
440+
value &&
441+
typeof value === 'object' &&
442+
typeof (value as Promise<T>).then === 'function',
443+
)
444+
}

‎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)