Skip to content

Commit d00b9c1

Browse files
refactor(router-core): fewer getMatch calls (#4971)
some miscellaneous minor optimizations in the `loadMatches` pipeline. - try and reduce the number of times we call `getMatch` - ~~`onReady` doesn't need to return a promise (because it's never used as such)~~ actually some things fail without the artificially added microtask here - don't create `beforeLoadPromise` if there is no `beforeLoad` option anyway <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Improved NotFound and error handling to prevent unnecessary blocking and ensure readiness triggers correctly. - More reliable SSR and preload behavior across route transitions. - Refactor - Streamlined route loading with per-route stale-while-revalidate, reducing redundant work and improving navigation responsiveness. - Reduced internal lookups and clarified readiness semantics for more predictable loading states. - Public API - onReady callback is now synchronous (no Promise), aligning with updated readiness flow. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent cb6d34a commit d00b9c1

File tree

1 file changed

+102
-112
lines changed

1 file changed

+102
-112
lines changed

packages/router-core/src/load-matches.ts

Lines changed: 102 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { isNotFound } from './not-found'
55
import { rootRouteId } from './root'
66
import { isRedirect } from './redirect'
77
import type { NotFoundError } from './not-found'
8-
import type { ControlledPromise } from './utils'
98
import type { ParsedLocation } from './location'
109
import type {
1110
AnyRoute,
@@ -56,12 +55,6 @@ const _handleNotFound = (inner: InnerLoadContext, err: NotFoundError) => {
5655
// First check if a specific route is requested to show the error
5756
const routeCursor =
5857
inner.router.routesById[err.routeId ?? ''] ?? inner.router.routeTree
59-
const matchesByRouteId: Record<string, AnyRouteMatch> = {}
60-
61-
// Setup routesByRouteId object for quick access
62-
for (const match of inner.matches) {
63-
matchesByRouteId[match.routeId] = match
64-
}
6558

6659
// Ensure a NotFoundComponent exists on the route
6760
if (
@@ -80,7 +73,7 @@ const _handleNotFound = (inner: InnerLoadContext, err: NotFoundError) => {
8073
)
8174

8275
// Find the match for this route
83-
const matchForRoute = matchesByRouteId[routeCursor.id]
76+
const matchForRoute = inner.matches.find((m) => m.routeId === routeCursor.id)
8477

8578
invariant(matchForRoute, 'Could not find match for route: ' + routeCursor.id)
8679

@@ -246,7 +239,7 @@ const isBeforeLoadSsr = (
246239
existingMatch.ssr = parentOverride(route.options.ssr)
247240
return
248241
}
249-
const { search, params } = inner.router.getMatch(matchId)!
242+
const { search, params } = existingMatch
250243

251244
const ssrFnContext: SsrContextOptions<any, any, any> = {
252245
search: makeMaybe(search, existingMatch.searchError),
@@ -280,8 +273,8 @@ const setupPendingTimeout = (
280273
inner: InnerLoadContext,
281274
matchId: string,
282275
route: AnyRoute,
276+
match: AnyRouteMatch,
283277
): void => {
284-
const match = inner.router.getMatch(matchId)!
285278
if (match._nonReactive.pendingTimeout !== undefined) return
286279

287280
const pendingMs =
@@ -324,20 +317,20 @@ const shouldExecuteBeforeLoad = (
324317
)
325318
return true
326319

327-
setupPendingTimeout(inner, matchId, route)
320+
setupPendingTimeout(inner, matchId, route, existingMatch)
328321

329322
const then = () => {
330-
let shouldExecuteBeforeLoad = true
323+
let result = true
331324
const match = inner.router.getMatch(matchId)!
332325
if (match.status === 'error') {
333-
shouldExecuteBeforeLoad = true
326+
result = true
334327
} else if (
335328
match.preload &&
336329
(match.status === 'redirected' || match.status === 'notFound')
337330
) {
338331
handleRedirectAndNotFound(inner, match, match.error)
339332
}
340-
return shouldExecuteBeforeLoad
333+
return result
341334
}
342335

343336
// Wait for the beforeLoad to resolve before we continue
@@ -354,7 +347,6 @@ const executeBeforeLoad = (
354347
): void | Promise<void> => {
355348
const match = inner.router.getMatch(matchId)!
356349

357-
match._nonReactive.beforeLoadPromise = createControlledPromise<void>()
358350
// explicitly capture the previous loadPromise
359351
const prevLoadPromise = match._nonReactive.loadPromise
360352
match._nonReactive.loadPromise = createControlledPromise<void>(() => {
@@ -371,7 +363,7 @@ const executeBeforeLoad = (
371363
handleSerialError(inner, index, searchError, 'VALIDATE_SEARCH')
372364
}
373365

374-
setupPendingTimeout(inner, matchId, route)
366+
setupPendingTimeout(inner, matchId, route, match)
375367

376368
const abortController = new AbortController()
377369

@@ -415,6 +407,8 @@ const executeBeforeLoad = (
415407
return
416408
}
417409

410+
match._nonReactive.beforeLoadPromise = createControlledPromise<void>()
411+
418412
const { search, params, cause } = match
419413
const preload = resolvePreload(inner, matchId)
420414
const beforeLoadFnContext: BeforeLoadContextOptions<any, any, any, any, any> =
@@ -510,8 +504,8 @@ const handleBeforeLoad = (
510504
: execute(shouldExecuteBeforeLoadResult)
511505
}
512506

513-
const execute = (shouldExecuteBeforeLoad: boolean) => {
514-
if (shouldExecuteBeforeLoad) {
507+
const execute = (shouldExec: boolean) => {
508+
if (shouldExec) {
515509
// If we are not in the middle of a load OR the previous load failed, start it
516510
return executeBeforeLoad(inner, matchId, index, route)
517511
}
@@ -567,14 +561,6 @@ const executeHead = (
567561
})
568562
}
569563

570-
const potentialPendingMinPromise = (
571-
inner: InnerLoadContext,
572-
matchId: string,
573-
): void | ControlledPromise<void> => {
574-
const latestMatch = inner.router.getMatch(matchId)!
575-
return latestMatch._nonReactive.minPendingPromise
576-
}
577-
578564
const getLoaderContext = (
579565
inner: InnerLoadContext,
580566
matchId: string,
@@ -592,7 +578,7 @@ const getLoaderContext = (
592578
deps: loaderDeps,
593579
preload: !!preload,
594580
parentMatchPromise,
595-
abortController: abortController,
581+
abortController,
596582
context,
597583
location: inner.location,
598584
navigate: (opts) =>
@@ -618,12 +604,11 @@ const runLoader = async (
618604
// before committing to the match and resolving
619605
// the loadPromise
620606

607+
const match = inner.router.getMatch(matchId)!
608+
621609
// Actually run the loader and handle the result
622610
try {
623-
if (
624-
!inner.router.isServer ||
625-
inner.router.getMatch(matchId)!.ssr === true
626-
) {
611+
if (!inner.router.isServer || match.ssr === true) {
627612
loadRouteChunk(route)
628613
}
629614

@@ -641,7 +626,7 @@ const runLoader = async (
641626
route.options.head ||
642627
route.options.scripts ||
643628
route.options.headers ||
644-
inner.router.getMatch(matchId)!._nonReactive.minPendingPromise
629+
match._nonReactive.minPendingPromise
645630
)
646631

647632
if (willLoadSomething) {
@@ -675,7 +660,7 @@ const runLoader = async (
675660
if (route._lazyPromise) await route._lazyPromise
676661
const headResult = executeHead(inner, matchId, route)
677662
const head = headResult ? await headResult : undefined
678-
const pendingPromise = potentialPendingMinPromise(inner, matchId)
663+
const pendingPromise = match._nonReactive.minPendingPromise
679664
if (pendingPromise) await pendingPromise
680665

681666
// Last but not least, wait for the the components
@@ -692,7 +677,7 @@ const runLoader = async (
692677
} catch (e) {
693678
let error = e
694679

695-
const pendingPromise = potentialPendingMinPromise(inner, matchId)
680+
const pendingPromise = match._nonReactive.minPendingPromise
696681
if (pendingPromise) await pendingPromise
697682

698683
handleRedirectAndNotFound(inner, inner.router.getMatch(matchId), e)
@@ -744,7 +729,6 @@ const loadRouteMatch = async (
744729
let loaderIsRunningAsync = false
745730
const route = inner.router.looseRoutesById[routeId]!
746731

747-
const prevMatch = inner.router.getMatch(matchId)!
748732
if (shouldSkipLoader(inner, matchId)) {
749733
if (inner.router.isServer) {
750734
const headResult = executeHead(inner, matchId, route)
@@ -757,88 +741,92 @@ const loadRouteMatch = async (
757741
}
758742
return inner.router.getMatch(matchId)!
759743
}
760-
}
761-
// there is a loaderPromise, so we are in the middle of a load
762-
else if (prevMatch._nonReactive.loaderPromise) {
763-
// do not block if we already have stale data we can show
764-
// but only if the ongoing load is not a preload since error handling is different for preloads
765-
// and we don't want to swallow errors
766-
if (prevMatch.status === 'success' && !inner.sync && !prevMatch.preload) {
767-
return inner.router.getMatch(matchId)!
768-
}
769-
await prevMatch._nonReactive.loaderPromise
770-
const match = inner.router.getMatch(matchId)!
771-
if (match.error) {
772-
handleRedirectAndNotFound(inner, match, match.error)
773-
}
774744
} else {
775-
// This is where all of the stale-while-revalidate magic happens
776-
const age = Date.now() - inner.router.getMatch(matchId)!.updatedAt
777-
778-
const preload = resolvePreload(inner, matchId)
779-
780-
const staleAge = preload
781-
? (route.options.preloadStaleTime ??
782-
inner.router.options.defaultPreloadStaleTime ??
783-
30_000) // 30 seconds for preloads by default
784-
: (route.options.staleTime ?? inner.router.options.defaultStaleTime ?? 0)
785-
786-
const shouldReloadOption = route.options.shouldReload
787-
788-
// Default to reloading the route all the time
789-
// Allow shouldReload to get the last say,
790-
// if provided.
791-
const shouldReload =
792-
typeof shouldReloadOption === 'function'
793-
? shouldReloadOption(getLoaderContext(inner, matchId, index, route))
794-
: shouldReloadOption
795-
796-
const nextPreload =
797-
!!preload && !inner.router.state.matches.some((d) => d.id === matchId)
798-
const match = inner.router.getMatch(matchId)!
799-
match._nonReactive.loaderPromise = createControlledPromise<void>()
800-
if (nextPreload !== match.preload) {
801-
inner.updateMatch(matchId, (prev) => ({
802-
...prev,
803-
preload: nextPreload,
804-
}))
805-
}
806-
807-
// If the route is successful and still fresh, just resolve
808-
const { status, invalid } = inner.router.getMatch(matchId)!
809-
loaderShouldRunAsync =
810-
status === 'success' && (invalid || (shouldReload ?? age > staleAge))
811-
if (preload && route.options.preload === false) {
812-
// Do nothing
813-
} else if (loaderShouldRunAsync && !inner.sync) {
814-
loaderIsRunningAsync = true
815-
;(async () => {
816-
try {
817-
await runLoader(inner, matchId, index, route)
818-
const match = inner.router.getMatch(matchId)!
819-
match._nonReactive.loaderPromise?.resolve()
820-
match._nonReactive.loadPromise?.resolve()
821-
match._nonReactive.loaderPromise = undefined
822-
} catch (err) {
823-
if (isRedirect(err)) {
824-
await inner.router.navigate(err.options)
825-
}
826-
}
827-
})()
828-
} else if (status !== 'success' || (loaderShouldRunAsync && inner.sync)) {
829-
await runLoader(inner, matchId, index, route)
745+
const prevMatch = inner.router.getMatch(matchId)!
746+
// there is a loaderPromise, so we are in the middle of a load
747+
if (prevMatch._nonReactive.loaderPromise) {
748+
// do not block if we already have stale data we can show
749+
// but only if the ongoing load is not a preload since error handling is different for preloads
750+
// and we don't want to swallow errors
751+
if (prevMatch.status === 'success' && !inner.sync && !prevMatch.preload) {
752+
return prevMatch
753+
}
754+
await prevMatch._nonReactive.loaderPromise
755+
const match = inner.router.getMatch(matchId)!
756+
if (match.error) {
757+
handleRedirectAndNotFound(inner, match, match.error)
758+
}
830759
} else {
831-
// if the loader did not run, still update head.
832-
// reason: parent's beforeLoad may have changed the route context
833-
// and only now do we know the route context (and that the loader would not run)
834-
const headResult = executeHead(inner, matchId, route)
835-
if (headResult) {
836-
const head = await headResult
760+
// This is where all of the stale-while-revalidate magic happens
761+
const age = Date.now() - prevMatch.updatedAt
762+
763+
const preload = resolvePreload(inner, matchId)
764+
765+
const staleAge = preload
766+
? (route.options.preloadStaleTime ??
767+
inner.router.options.defaultPreloadStaleTime ??
768+
30_000) // 30 seconds for preloads by default
769+
: (route.options.staleTime ??
770+
inner.router.options.defaultStaleTime ??
771+
0)
772+
773+
const shouldReloadOption = route.options.shouldReload
774+
775+
// Default to reloading the route all the time
776+
// Allow shouldReload to get the last say,
777+
// if provided.
778+
const shouldReload =
779+
typeof shouldReloadOption === 'function'
780+
? shouldReloadOption(getLoaderContext(inner, matchId, index, route))
781+
: shouldReloadOption
782+
783+
const nextPreload =
784+
!!preload && !inner.router.state.matches.some((d) => d.id === matchId)
785+
const match = inner.router.getMatch(matchId)!
786+
match._nonReactive.loaderPromise = createControlledPromise<void>()
787+
if (nextPreload !== match.preload) {
837788
inner.updateMatch(matchId, (prev) => ({
838789
...prev,
839-
...head,
790+
preload: nextPreload,
840791
}))
841792
}
793+
794+
// If the route is successful and still fresh, just resolve
795+
const { status, invalid } = match
796+
loaderShouldRunAsync =
797+
status === 'success' && (invalid || (shouldReload ?? age > staleAge))
798+
if (preload && route.options.preload === false) {
799+
// Do nothing
800+
} else if (loaderShouldRunAsync && !inner.sync) {
801+
loaderIsRunningAsync = true
802+
;(async () => {
803+
try {
804+
await runLoader(inner, matchId, index, route)
805+
const match = inner.router.getMatch(matchId)!
806+
match._nonReactive.loaderPromise?.resolve()
807+
match._nonReactive.loadPromise?.resolve()
808+
match._nonReactive.loaderPromise = undefined
809+
} catch (err) {
810+
if (isRedirect(err)) {
811+
await inner.router.navigate(err.options)
812+
}
813+
}
814+
})()
815+
} else if (status !== 'success' || (loaderShouldRunAsync && inner.sync)) {
816+
await runLoader(inner, matchId, index, route)
817+
} else {
818+
// if the loader did not run, still update head.
819+
// reason: parent's beforeLoad may have changed the route context
820+
// and only now do we know the route context (and that the loader would not run)
821+
const headResult = executeHead(inner, matchId, route)
822+
if (headResult) {
823+
const head = await headResult
824+
inner.updateMatch(matchId, (prev) => ({
825+
...prev,
826+
...head,
827+
}))
828+
}
829+
}
842830
}
843831
}
844832
const match = inner.router.getMatch(matchId)!
@@ -858,8 +846,10 @@ const loadRouteMatch = async (
858846
isFetching: nextIsFetching,
859847
invalid: false,
860848
}))
849+
return inner.router.getMatch(matchId)!
850+
} else {
851+
return match
861852
}
862-
return inner.router.getMatch(matchId)!
863853
}
864854

865855
export async function loadMatches(arg: {

0 commit comments

Comments
 (0)