From 1720f28a5fc3d52035dcd89fbfd07ca2a6ab779f Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sat, 16 Aug 2025 20:06:13 +0200 Subject: [PATCH 1/3] refactor(router-core): fewer getMatch calls --- packages/router-core/src/load-matches.ts | 226 +++++++++++------------ packages/router-core/src/router.ts | 3 +- 2 files changed, 108 insertions(+), 121 deletions(-) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 6940488cab2..dc30e697701 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -5,7 +5,6 @@ import { isNotFound } from './not-found' import { rootRouteId } from './root' import { isRedirect } from './redirect' import type { NotFoundError } from './not-found' -import type { ControlledPromise } from './utils' import type { ParsedLocation } from './location' import type { AnyRoute, @@ -32,13 +31,13 @@ type InnerLoadContext = { updateMatch: UpdateMatchFn matches: Array preload?: boolean - onReady?: () => Promise + onReady?: () => void sync?: boolean /** mutable state, scoped to a `loadMatches` call */ matchPromises: Array> } -const triggerOnReady = (inner: InnerLoadContext): void | Promise => { +const triggerOnReady = (inner: InnerLoadContext): void => { if (!inner.rendered) { inner.rendered = true return inner.onReady?.() @@ -56,12 +55,6 @@ const _handleNotFound = (inner: InnerLoadContext, err: NotFoundError) => { // First check if a specific route is requested to show the error const routeCursor = inner.router.routesById[err.routeId ?? ''] ?? inner.router.routeTree - const matchesByRouteId: Record = {} - - // Setup routesByRouteId object for quick access - for (const match of inner.matches) { - matchesByRouteId[match.routeId] = match - } // Ensure a NotFoundComponent exists on the route if ( @@ -80,7 +73,7 @@ const _handleNotFound = (inner: InnerLoadContext, err: NotFoundError) => { ) // Find the match for this route - const matchForRoute = matchesByRouteId[routeCursor.id] + const matchForRoute = inner.matches.find((m) => m.routeId === routeCursor.id) invariant(matchForRoute, 'Could not find match for route: ' + routeCursor.id) @@ -246,7 +239,7 @@ const isBeforeLoadSsr = ( existingMatch.ssr = parentOverride(route.options.ssr) return } - const { search, params } = inner.router.getMatch(matchId)! + const { search, params } = existingMatch const ssrFnContext: SsrContextOptions = { search: makeMaybe(search, existingMatch.searchError), @@ -280,8 +273,8 @@ const setupPendingTimeout = ( inner: InnerLoadContext, matchId: string, route: AnyRoute, + match: AnyRouteMatch, ): void => { - const match = inner.router.getMatch(matchId)! if (match._nonReactive.pendingTimeout !== undefined) return const pendingMs = @@ -324,20 +317,20 @@ const shouldExecuteBeforeLoad = ( ) return true - setupPendingTimeout(inner, matchId, route) + setupPendingTimeout(inner, matchId, route, existingMatch) const then = () => { - let shouldExecuteBeforeLoad = true + let result = true const match = inner.router.getMatch(matchId)! if (match.status === 'error') { - shouldExecuteBeforeLoad = true + result = true } else if ( match.preload && (match.status === 'redirected' || match.status === 'notFound') ) { handleRedirectAndNotFound(inner, match, match.error) } - return shouldExecuteBeforeLoad + return result } // Wait for the beforeLoad to resolve before we continue @@ -354,7 +347,6 @@ const executeBeforeLoad = ( ): void | Promise => { const match = inner.router.getMatch(matchId)! - match._nonReactive.beforeLoadPromise = createControlledPromise() // explicitly capture the previous loadPromise const prevLoadPromise = match._nonReactive.loadPromise match._nonReactive.loadPromise = createControlledPromise(() => { @@ -371,7 +363,7 @@ const executeBeforeLoad = ( handleSerialError(inner, index, searchError, 'VALIDATE_SEARCH') } - setupPendingTimeout(inner, matchId, route) + setupPendingTimeout(inner, matchId, route, match) const abortController = new AbortController() @@ -415,6 +407,8 @@ const executeBeforeLoad = ( return } + match._nonReactive.beforeLoadPromise = createControlledPromise() + const { search, params, cause } = match const preload = resolvePreload(inner, matchId) const beforeLoadFnContext: BeforeLoadContextOptions = @@ -510,8 +504,8 @@ const handleBeforeLoad = ( : execute(shouldExecuteBeforeLoadResult) } - const execute = (shouldExecuteBeforeLoad: boolean) => { - if (shouldExecuteBeforeLoad) { + const execute = (shouldExec: boolean) => { + if (shouldExec) { // If we are not in the middle of a load OR the previous load failed, start it return executeBeforeLoad(inner, matchId, index, route) } @@ -567,14 +561,6 @@ const executeHead = ( }) } -const potentialPendingMinPromise = ( - inner: InnerLoadContext, - matchId: string, -): void | ControlledPromise => { - const latestMatch = inner.router.getMatch(matchId)! - return latestMatch._nonReactive.minPendingPromise -} - const getLoaderContext = ( inner: InnerLoadContext, matchId: string, @@ -592,7 +578,7 @@ const getLoaderContext = ( deps: loaderDeps, preload: !!preload, parentMatchPromise, - abortController: abortController, + abortController, context, location: inner.location, navigate: (opts) => @@ -618,12 +604,11 @@ const runLoader = async ( // before committing to the match and resolving // the loadPromise + const match = inner.router.getMatch(matchId)! + // Actually run the loader and handle the result try { - if ( - !inner.router.isServer || - inner.router.getMatch(matchId)!.ssr === true - ) { + if (!inner.router.isServer || match.ssr === true) { loadRouteChunk(route) } @@ -641,7 +626,7 @@ const runLoader = async ( route.options.head || route.options.scripts || route.options.headers || - inner.router.getMatch(matchId)!._nonReactive.minPendingPromise + match._nonReactive.minPendingPromise ) if (willLoadSomething) { @@ -675,7 +660,7 @@ const runLoader = async ( if (route._lazyPromise) await route._lazyPromise const headResult = executeHead(inner, matchId, route) const head = headResult ? await headResult : undefined - const pendingPromise = potentialPendingMinPromise(inner, matchId) + const pendingPromise = match._nonReactive.minPendingPromise if (pendingPromise) await pendingPromise // Last but not least, wait for the the components @@ -692,7 +677,7 @@ const runLoader = async ( } catch (e) { let error = e - const pendingPromise = potentialPendingMinPromise(inner, matchId) + const pendingPromise = match._nonReactive.minPendingPromise if (pendingPromise) await pendingPromise handleRedirectAndNotFound(inner, inner.router.getMatch(matchId), e) @@ -744,7 +729,6 @@ const loadRouteMatch = async ( let loaderIsRunningAsync = false const route = inner.router.looseRoutesById[routeId]! - const prevMatch = inner.router.getMatch(matchId)! if (shouldSkipLoader(inner, matchId)) { if (inner.router.isServer) { const headResult = executeHead(inner, matchId, route) @@ -757,88 +741,92 @@ const loadRouteMatch = async ( } return inner.router.getMatch(matchId)! } - } - // there is a loaderPromise, so we are in the middle of a load - else if (prevMatch._nonReactive.loaderPromise) { - // do not block if we already have stale data we can show - // but only if the ongoing load is not a preload since error handling is different for preloads - // and we don't want to swallow errors - if (prevMatch.status === 'success' && !inner.sync && !prevMatch.preload) { - return inner.router.getMatch(matchId)! - } - await prevMatch._nonReactive.loaderPromise - const match = inner.router.getMatch(matchId)! - if (match.error) { - handleRedirectAndNotFound(inner, match, match.error) - } } else { - // This is where all of the stale-while-revalidate magic happens - const age = Date.now() - inner.router.getMatch(matchId)!.updatedAt - - const preload = resolvePreload(inner, matchId) - - const staleAge = preload - ? (route.options.preloadStaleTime ?? - inner.router.options.defaultPreloadStaleTime ?? - 30_000) // 30 seconds for preloads by default - : (route.options.staleTime ?? inner.router.options.defaultStaleTime ?? 0) - - const shouldReloadOption = route.options.shouldReload - - // Default to reloading the route all the time - // Allow shouldReload to get the last say, - // if provided. - const shouldReload = - typeof shouldReloadOption === 'function' - ? shouldReloadOption(getLoaderContext(inner, matchId, index, route)) - : shouldReloadOption - - const nextPreload = - !!preload && !inner.router.state.matches.some((d) => d.id === matchId) - const match = inner.router.getMatch(matchId)! - match._nonReactive.loaderPromise = createControlledPromise() - if (nextPreload !== match.preload) { - inner.updateMatch(matchId, (prev) => ({ - ...prev, - preload: nextPreload, - })) - } - - // If the route is successful and still fresh, just resolve - const { status, invalid } = inner.router.getMatch(matchId)! - loaderShouldRunAsync = - status === 'success' && (invalid || (shouldReload ?? age > staleAge)) - if (preload && route.options.preload === false) { - // Do nothing - } else if (loaderShouldRunAsync && !inner.sync) { - loaderIsRunningAsync = true - ;(async () => { - try { - await runLoader(inner, matchId, index, route) - const match = inner.router.getMatch(matchId)! - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.loadPromise?.resolve() - match._nonReactive.loaderPromise = undefined - } catch (err) { - if (isRedirect(err)) { - await inner.router.navigate(err.options) - } - } - })() - } else if (status !== 'success' || (loaderShouldRunAsync && inner.sync)) { - await runLoader(inner, matchId, index, route) + const prevMatch = inner.router.getMatch(matchId)! + // there is a loaderPromise, so we are in the middle of a load + if (prevMatch._nonReactive.loaderPromise) { + // do not block if we already have stale data we can show + // but only if the ongoing load is not a preload since error handling is different for preloads + // and we don't want to swallow errors + if (prevMatch.status === 'success' && !inner.sync && !prevMatch.preload) { + return prevMatch + } + await prevMatch._nonReactive.loaderPromise + const match = inner.router.getMatch(matchId)! + if (match.error) { + handleRedirectAndNotFound(inner, match, match.error) + } } else { - // if the loader did not run, still update head. - // reason: parent's beforeLoad may have changed the route context - // and only now do we know the route context (and that the loader would not run) - const headResult = executeHead(inner, matchId, route) - if (headResult) { - const head = await headResult + // This is where all of the stale-while-revalidate magic happens + const age = Date.now() - prevMatch.updatedAt + + const preload = resolvePreload(inner, matchId) + + const staleAge = preload + ? (route.options.preloadStaleTime ?? + inner.router.options.defaultPreloadStaleTime ?? + 30_000) // 30 seconds for preloads by default + : (route.options.staleTime ?? + inner.router.options.defaultStaleTime ?? + 0) + + const shouldReloadOption = route.options.shouldReload + + // Default to reloading the route all the time + // Allow shouldReload to get the last say, + // if provided. + const shouldReload = + typeof shouldReloadOption === 'function' + ? shouldReloadOption(getLoaderContext(inner, matchId, index, route)) + : shouldReloadOption + + const nextPreload = + !!preload && !inner.router.state.matches.some((d) => d.id === matchId) + const match = inner.router.getMatch(matchId)! + match._nonReactive.loaderPromise = createControlledPromise() + if (nextPreload !== match.preload) { inner.updateMatch(matchId, (prev) => ({ ...prev, - ...head, + preload: nextPreload, })) } + + // If the route is successful and still fresh, just resolve + const { status, invalid } = match + loaderShouldRunAsync = + status === 'success' && (invalid || (shouldReload ?? age > staleAge)) + if (preload && route.options.preload === false) { + // Do nothing + } else if (loaderShouldRunAsync && !inner.sync) { + loaderIsRunningAsync = true + ;(async () => { + try { + await runLoader(inner, matchId, index, route) + const match = inner.router.getMatch(matchId)! + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.loadPromise?.resolve() + match._nonReactive.loaderPromise = undefined + } catch (err) { + if (isRedirect(err)) { + await inner.router.navigate(err.options) + } + } + })() + } else if (status !== 'success' || (loaderShouldRunAsync && inner.sync)) { + await runLoader(inner, matchId, index, route) + } else { + // if the loader did not run, still update head. + // reason: parent's beforeLoad may have changed the route context + // and only now do we know the route context (and that the loader would not run) + const headResult = executeHead(inner, matchId, route) + if (headResult) { + const head = await headResult + inner.updateMatch(matchId, (prev) => ({ + ...prev, + ...head, + })) + } + } } } const match = inner.router.getMatch(matchId)! @@ -858,8 +846,10 @@ const loadRouteMatch = async ( isFetching: nextIsFetching, invalid: false, })) + return inner.router.getMatch(matchId)! + } else { + return match } - return inner.router.getMatch(matchId)! } export async function loadMatches(arg: { @@ -867,7 +857,7 @@ export async function loadMatches(arg: { location: ParsedLocation matches: Array preload?: boolean - onReady?: () => Promise + onReady?: () => void updateMatch: UpdateMatchFn sync?: boolean }): Promise> { @@ -898,12 +888,10 @@ export async function loadMatches(arg: { } await Promise.all(inner.matchPromises) - const readyPromise = triggerOnReady(inner) - if (isPromise(readyPromise)) await readyPromise + triggerOnReady(inner) } catch (err) { if (isNotFound(err) && !inner.preload) { - const readyPromise = triggerOnReady(inner) - if (isPromise(readyPromise)) await readyPromise + triggerOnReady(inner) throw err } if (isRedirect(err)) { diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index c4409961b83..9d1a20a1e9d 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1893,8 +1893,7 @@ export class RouterCore< matches: this.state.pendingMatches as Array, location: next, updateMatch: this.updateMatch, - // eslint-disable-next-line @typescript-eslint/require-await - onReady: async () => { + onReady: () => { // eslint-disable-next-line @typescript-eslint/require-await this.startViewTransition(async () => { // this.viewTransitionPromise = createControlledPromise() From 179830b53afce625e72dc4871acba48e449409e9 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sat, 16 Aug 2025 20:33:59 +0200 Subject: [PATCH 2/3] keep onReady promise --- packages/router-core/src/load-matches.ts | 12 +++++++----- packages/router-core/src/router.ts | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index dc30e697701..2ce8c3bc2c2 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -31,13 +31,13 @@ type InnerLoadContext = { updateMatch: UpdateMatchFn matches: Array preload?: boolean - onReady?: () => void + onReady?: () => Promise sync?: boolean /** mutable state, scoped to a `loadMatches` call */ matchPromises: Array> } -const triggerOnReady = (inner: InnerLoadContext): void => { +const triggerOnReady = (inner: InnerLoadContext): void | Promise => { if (!inner.rendered) { inner.rendered = true return inner.onReady?.() @@ -857,7 +857,7 @@ export async function loadMatches(arg: { location: ParsedLocation matches: Array preload?: boolean - onReady?: () => void + onReady?: () => Promise updateMatch: UpdateMatchFn sync?: boolean }): Promise> { @@ -888,10 +888,12 @@ export async function loadMatches(arg: { } await Promise.all(inner.matchPromises) - triggerOnReady(inner) + const readyPromise = triggerOnReady(inner) + if (isPromise(readyPromise)) await readyPromise } catch (err) { if (isNotFound(err) && !inner.preload) { - triggerOnReady(inner) + const readyPromise = triggerOnReady(inner) + if (isPromise(readyPromise)) await readyPromise throw err } if (isRedirect(err)) { diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 9d1a20a1e9d..c4409961b83 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1893,7 +1893,8 @@ export class RouterCore< matches: this.state.pendingMatches as Array, location: next, updateMatch: this.updateMatch, - onReady: () => { + // eslint-disable-next-line @typescript-eslint/require-await + onReady: async () => { // eslint-disable-next-line @typescript-eslint/require-await this.startViewTransition(async () => { // this.viewTransitionPromise = createControlledPromise() From 0ec2d48b72785a609dca6da6e6d345664c02f35d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 16 Aug 2025 18:34:49 +0000 Subject: [PATCH 3/3] ci: apply automated fixes --- packages/router-core/src/load-matches.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 2ce8c3bc2c2..3abfbe1f5d6 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -888,7 +888,7 @@ export async function loadMatches(arg: { } await Promise.all(inner.matchPromises) - const readyPromise = triggerOnReady(inner) + const readyPromise = triggerOnReady(inner) if (isPromise(readyPromise)) await readyPromise } catch (err) { if (isNotFound(err) && !inner.preload) {