From 00e4a8ab10cbdaeaed63a64d2431f073f780cc69 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Mon, 11 Aug 2025 20:02:46 +0200 Subject: [PATCH 01/13] refactor(router-core): head, scripts, headers are executed in parallel and can skip store update --- packages/router-core/src/router.ts | 203 ++++++++++++++++------------- 1 file changed, 110 insertions(+), 93 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 58b996d866..9dc0df30b6 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -108,10 +108,10 @@ export type DefaultRemountDepsFn = ( opts: MakeRemountDepsOptionsUnion, ) => any -export interface DefaultRouterOptionsExtensions {} +export interface DefaultRouterOptionsExtensions { } export interface RouterOptionsExtensions - extends DefaultRouterOptionsExtensions {} + extends DefaultRouterOptionsExtensions { } export interface RouterOptions< TRouteTree extends AnyRoute, @@ -518,12 +518,12 @@ export type InferRouterContext = export type RouterContextOptions = AnyContext extends InferRouterContext - ? { - context?: InferRouterContext - } - : { - context: InferRouterContext - } + ? { + context?: InferRouterContext + } + : { + context: InferRouterContext + } export type RouterConstructorOptions< TRouteTree extends AnyRoute, @@ -695,14 +695,14 @@ export type AnyRouter = RouterCore export interface ViewTransitionOptions { types: - | Array - | ((locationChangeInfo: { - fromLocation?: ParsedLocation - toLocation: ParsedLocation - pathChanged: boolean - hrefChanged: boolean - hashChanged: boolean - }) => Array) + | Array + | ((locationChangeInfo: { + fromLocation?: ParsedLocation + toLocation: ParsedLocation + pathChanged: boolean + hrefChanged: boolean + hashChanged: boolean + }) => Array) } export function defaultSerializeError(err: unknown) { @@ -713,7 +713,7 @@ export function defaultSerializeError(err: unknown) { } if (process.env.NODE_ENV === 'development') { - ;(obj as any).stack = err.stack + ; (obj as any).stack = err.stack } return obj @@ -748,12 +748,12 @@ export type CreateRouterFn = < options: undefined extends number ? 'strictNullChecks must be enabled in tsconfig.json' : RouterConstructorOptions< - TRouteTree, - TTrailingSlashOption, - TDefaultStructuralSharingOption, - TRouterHistory, - TDehydrated - >, + TRouteTree, + TTrailingSlashOption, + TDefaultStructuralSharingOption, + TRouterHistory, + TDehydrated + >, ) => RouterCore< TRouteTree, TTrailingSlashOption, @@ -868,11 +868,11 @@ export class RouterCore< this.pathParamsDecodeCharMap = this.options.pathParamsAllowedCharacters ? new Map( - this.options.pathParamsAllowedCharacters.map((char) => [ - encodeURIComponent(char), - char, - ]), - ) + this.options.pathParamsAllowedCharacters.map((char) => [ + encodeURIComponent(char), + char, + ]), + ) : undefined if ( @@ -898,8 +898,8 @@ export class RouterCore< this.options.history ?? ((this.isServer ? createMemoryHistory({ - initialEntries: [this.basepath || '/'], - }) + initialEntries: [this.basepath || '/'], + }) : createBrowserHistory()) as TRouterHistory) this.updateLatestLocation() } @@ -1085,7 +1085,7 @@ export class RouterCore< foundRoute ? foundRoute.path !== '/' && routeParams['**'] : // Or if we didn't find a route and we have left over path - trimPathRight(next.pathname) + trimPathRight(next.pathname) ) { // If the user has defined an (old) 404 route, use it if (this.options.notFoundRoute) { @@ -1259,9 +1259,9 @@ export class RouterCore< } else { const status = route.options.loader || - route.options.beforeLoad || - route.lazyFn || - routeNeedsPreload(route) + route.options.beforeLoad || + route.lazyFn || + routeNeedsPreload(route) ? 'pending' : 'success' @@ -1478,9 +1478,9 @@ export class RouterCore< : (dest.params ?? true) === true ? fromParams : { - ...fromParams, - ...functionalUpdate(dest.params as any, fromParams), - } + ...fromParams, + ...functionalUpdate(dest.params as any, fromParams), + } // Interpolate the path first to get the actual resolved path, then match against that const interpolatedNextTo = interpolatePath({ @@ -1665,7 +1665,7 @@ export class RouterCore< '__hashScrollIntoViewOptions', ] as const ignoredProps.forEach((prop) => { - ;(next.state as any)[prop] = this.latestLocation.state[prop] + ; (next.state as any)[prop] = this.latestLocation.state[prop] }) const isEqual = deepEqual(next.state, this.latestLocation.state) ignoredProps.forEach((prop) => { @@ -1779,7 +1779,7 @@ export class RouterCore< try { new URL(`${href}`) reloadDocument = true - } catch {} + } catch { } } if (reloadDocument) { @@ -1934,18 +1934,18 @@ export class RouterCore< this.clearExpiredCache() }) - // - ;( - [ - [exitingMatches, 'onLeave'], - [enteringMatches, 'onEnter'], - [stayingMatches, 'onStay'], - ] as const - ).forEach(([matches, hook]) => { - matches.forEach((match) => { - this.looseRoutesById[match.routeId]!.options[hook]?.(match) + // + ; ( + [ + [exitingMatches, 'onLeave'], + [enteringMatches, 'onEnter'], + [stayingMatches, 'onStay'], + ] as const + ).forEach(([matches, hook]) => { + matches.forEach((match) => { + this.looseRoutesById[match.routeId]!.options[hook]?.(match) + }) }) - }) }) }, }) @@ -2033,11 +2033,11 @@ export class RouterCore< const resolvedViewTransitionTypes = typeof shouldViewTransition.types === 'function' ? shouldViewTransition.types( - getLocationChangeInfo({ - resolvedLocation: prevLocation, - location: next, - }), - ) + getLocationChangeInfo({ + resolvedLocation: prevLocation, + location: next, + }), + ) : shouldViewTransition.types startViewTransitionParams = { @@ -2146,7 +2146,7 @@ export class RouterCore< })) if (!(err as any).routeId) { - ;(err as any).routeId = match.routeId + ; (err as any).routeId = match.routeId } match._nonReactive.loadPromise?.resolve() @@ -2183,7 +2183,7 @@ export class RouterCore< try { await new Promise((resolveAll, rejectAll) => { - ;(async () => { + ; (async () => { try { const handleSerialError = ( index: number, @@ -2324,7 +2324,7 @@ export class RouterCore< // Update the match and prematurely resolve the loadMatches promise so that // the pending component can start rendering triggerOnReady() - } catch {} + } catch { } }, pendingMs) match._nonReactive.pendingTimeout = pendingTimeout } @@ -2469,21 +2469,33 @@ export class RouterCore< if (!match) { return } + if ( + !route.options.head && + !route.options.scripts && + !route.options.headers + ) { + return + } const assetContext = { matches, match, params: match.params, loaderData: match.loaderData, } - const headFnContent = - await route.options.head?.(assetContext) + + const [headFnContent, scripts, headers] = await Promise.all( + [ + route.options.head?.(assetContext), + route.options.scripts?.(assetContext), + route.options.headers?.(assetContext), + ], + ) + const meta = headFnContent?.meta const links = headFnContent?.links const headScripts = headFnContent?.scripts const styles = headFnContent?.styles - const scripts = await route.options.scripts?.(assetContext) - const headers = await route.options.headers?.(assetContext) return { meta, links, @@ -2505,10 +2517,12 @@ export class RouterCore< if (shouldSkipLoader(matchId)) { if (this.isServer) { const head = await executeHead() - updateMatch(matchId, (prev) => ({ - ...prev, - ...head, - })) + if (head) { + updateMatch(matchId, (prev) => ({ + ...prev, + ...head, + })) + } return this.getMatch(matchId)! } } @@ -2674,14 +2688,15 @@ export class RouterCore< } catch (err) { const head = await executeHead() - updateMatch(matchId, (prev) => { - prev._nonReactive.loaderPromise = undefined - return { + if (head) { + updateMatch(matchId, (prev) => ({ ...prev, ...head, - } - }) - handleRedirectAndNotFound(this.getMatch(matchId)!, err) + })) + } + const match = this.getMatch(matchId)! + match._nonReactive.loaderPromise = undefined + handleRedirectAndNotFound(match, err) } } @@ -2694,19 +2709,19 @@ export class RouterCore< // Do nothing } else if (loaderShouldRunAsync && !sync) { loaderIsRunningAsync = true - ;(async () => { - try { - await runLoader() - const match = this.getMatch(matchId)! - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.loadPromise?.resolve() - match._nonReactive.loaderPromise = undefined - } catch (err) { - if (isRedirect(err)) { - await this.navigate(err.options) + ; (async () => { + try { + await runLoader() + const match = this.getMatch(matchId)! + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.loadPromise?.resolve() + match._nonReactive.loaderPromise = undefined + } catch (err) { + if (isRedirect(err)) { + await this.navigate(err.options) + } } - } - })() + })() } else if ( status !== 'success' || (loaderShouldRunAsync && sync) @@ -2717,10 +2732,12 @@ export class RouterCore< // 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 head = await executeHead() - updateMatch(matchId, (prev) => ({ - ...prev, - ...head, - })) + if (head) { + updateMatch(matchId, (prev) => ({ + ...prev, + ...head, + })) + } } } if (!loaderIsRunningAsync) { @@ -2976,9 +2993,9 @@ export class RouterCore< ...location, to: location.to ? this.resolvePathWithBase( - (location.from || '') as string, - location.to as string, - ) + (location.from || '') as string, + location.to as string, + ) : undefined, params: location.params || {}, leaveParams: true, @@ -3099,9 +3116,9 @@ export class RouterCore< } } -export class SearchParamError extends Error {} +export class SearchParamError extends Error { } -export class PathParamError extends Error {} +export class PathParamError extends Error { } const normalize = (str: string) => str.endsWith('/') && str.length > 1 ? str.slice(0, -1) : str From 265bd216eebff4ba0575d78a6af3e216d0013645 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 11 Aug 2025 18:05:47 +0000 Subject: [PATCH 02/13] ci: apply automated fixes --- packages/router-core/src/router.ts | 150 ++++++++++++++--------------- 1 file changed, 75 insertions(+), 75 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 9dc0df30b6..674c977319 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -108,10 +108,10 @@ export type DefaultRemountDepsFn = ( opts: MakeRemountDepsOptionsUnion, ) => any -export interface DefaultRouterOptionsExtensions { } +export interface DefaultRouterOptionsExtensions {} export interface RouterOptionsExtensions - extends DefaultRouterOptionsExtensions { } + extends DefaultRouterOptionsExtensions {} export interface RouterOptions< TRouteTree extends AnyRoute, @@ -518,12 +518,12 @@ export type InferRouterContext = export type RouterContextOptions = AnyContext extends InferRouterContext - ? { - context?: InferRouterContext - } - : { - context: InferRouterContext - } + ? { + context?: InferRouterContext + } + : { + context: InferRouterContext + } export type RouterConstructorOptions< TRouteTree extends AnyRoute, @@ -695,14 +695,14 @@ export type AnyRouter = RouterCore export interface ViewTransitionOptions { types: - | Array - | ((locationChangeInfo: { - fromLocation?: ParsedLocation - toLocation: ParsedLocation - pathChanged: boolean - hrefChanged: boolean - hashChanged: boolean - }) => Array) + | Array + | ((locationChangeInfo: { + fromLocation?: ParsedLocation + toLocation: ParsedLocation + pathChanged: boolean + hrefChanged: boolean + hashChanged: boolean + }) => Array) } export function defaultSerializeError(err: unknown) { @@ -713,7 +713,7 @@ export function defaultSerializeError(err: unknown) { } if (process.env.NODE_ENV === 'development') { - ; (obj as any).stack = err.stack + ;(obj as any).stack = err.stack } return obj @@ -748,12 +748,12 @@ export type CreateRouterFn = < options: undefined extends number ? 'strictNullChecks must be enabled in tsconfig.json' : RouterConstructorOptions< - TRouteTree, - TTrailingSlashOption, - TDefaultStructuralSharingOption, - TRouterHistory, - TDehydrated - >, + TRouteTree, + TTrailingSlashOption, + TDefaultStructuralSharingOption, + TRouterHistory, + TDehydrated + >, ) => RouterCore< TRouteTree, TTrailingSlashOption, @@ -868,11 +868,11 @@ export class RouterCore< this.pathParamsDecodeCharMap = this.options.pathParamsAllowedCharacters ? new Map( - this.options.pathParamsAllowedCharacters.map((char) => [ - encodeURIComponent(char), - char, - ]), - ) + this.options.pathParamsAllowedCharacters.map((char) => [ + encodeURIComponent(char), + char, + ]), + ) : undefined if ( @@ -898,8 +898,8 @@ export class RouterCore< this.options.history ?? ((this.isServer ? createMemoryHistory({ - initialEntries: [this.basepath || '/'], - }) + initialEntries: [this.basepath || '/'], + }) : createBrowserHistory()) as TRouterHistory) this.updateLatestLocation() } @@ -1085,7 +1085,7 @@ export class RouterCore< foundRoute ? foundRoute.path !== '/' && routeParams['**'] : // Or if we didn't find a route and we have left over path - trimPathRight(next.pathname) + trimPathRight(next.pathname) ) { // If the user has defined an (old) 404 route, use it if (this.options.notFoundRoute) { @@ -1259,9 +1259,9 @@ export class RouterCore< } else { const status = route.options.loader || - route.options.beforeLoad || - route.lazyFn || - routeNeedsPreload(route) + route.options.beforeLoad || + route.lazyFn || + routeNeedsPreload(route) ? 'pending' : 'success' @@ -1478,9 +1478,9 @@ export class RouterCore< : (dest.params ?? true) === true ? fromParams : { - ...fromParams, - ...functionalUpdate(dest.params as any, fromParams), - } + ...fromParams, + ...functionalUpdate(dest.params as any, fromParams), + } // Interpolate the path first to get the actual resolved path, then match against that const interpolatedNextTo = interpolatePath({ @@ -1665,7 +1665,7 @@ export class RouterCore< '__hashScrollIntoViewOptions', ] as const ignoredProps.forEach((prop) => { - ; (next.state as any)[prop] = this.latestLocation.state[prop] + ;(next.state as any)[prop] = this.latestLocation.state[prop] }) const isEqual = deepEqual(next.state, this.latestLocation.state) ignoredProps.forEach((prop) => { @@ -1779,7 +1779,7 @@ export class RouterCore< try { new URL(`${href}`) reloadDocument = true - } catch { } + } catch {} } if (reloadDocument) { @@ -1934,18 +1934,18 @@ export class RouterCore< this.clearExpiredCache() }) - // - ; ( - [ - [exitingMatches, 'onLeave'], - [enteringMatches, 'onEnter'], - [stayingMatches, 'onStay'], - ] as const - ).forEach(([matches, hook]) => { - matches.forEach((match) => { - this.looseRoutesById[match.routeId]!.options[hook]?.(match) - }) + // + ;( + [ + [exitingMatches, 'onLeave'], + [enteringMatches, 'onEnter'], + [stayingMatches, 'onStay'], + ] as const + ).forEach(([matches, hook]) => { + matches.forEach((match) => { + this.looseRoutesById[match.routeId]!.options[hook]?.(match) }) + }) }) }, }) @@ -2033,11 +2033,11 @@ export class RouterCore< const resolvedViewTransitionTypes = typeof shouldViewTransition.types === 'function' ? shouldViewTransition.types( - getLocationChangeInfo({ - resolvedLocation: prevLocation, - location: next, - }), - ) + getLocationChangeInfo({ + resolvedLocation: prevLocation, + location: next, + }), + ) : shouldViewTransition.types startViewTransitionParams = { @@ -2146,7 +2146,7 @@ export class RouterCore< })) if (!(err as any).routeId) { - ; (err as any).routeId = match.routeId + ;(err as any).routeId = match.routeId } match._nonReactive.loadPromise?.resolve() @@ -2183,7 +2183,7 @@ export class RouterCore< try { await new Promise((resolveAll, rejectAll) => { - ; (async () => { + ;(async () => { try { const handleSerialError = ( index: number, @@ -2324,7 +2324,7 @@ export class RouterCore< // Update the match and prematurely resolve the loadMatches promise so that // the pending component can start rendering triggerOnReady() - } catch { } + } catch {} }, pendingMs) match._nonReactive.pendingTimeout = pendingTimeout } @@ -2709,19 +2709,19 @@ export class RouterCore< // Do nothing } else if (loaderShouldRunAsync && !sync) { loaderIsRunningAsync = true - ; (async () => { - try { - await runLoader() - const match = this.getMatch(matchId)! - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.loadPromise?.resolve() - match._nonReactive.loaderPromise = undefined - } catch (err) { - if (isRedirect(err)) { - await this.navigate(err.options) - } + ;(async () => { + try { + await runLoader() + const match = this.getMatch(matchId)! + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.loadPromise?.resolve() + match._nonReactive.loaderPromise = undefined + } catch (err) { + if (isRedirect(err)) { + await this.navigate(err.options) } - })() + } + })() } else if ( status !== 'success' || (loaderShouldRunAsync && sync) @@ -2993,9 +2993,9 @@ export class RouterCore< ...location, to: location.to ? this.resolvePathWithBase( - (location.from || '') as string, - location.to as string, - ) + (location.from || '') as string, + location.to as string, + ) : undefined, params: location.params || {}, leaveParams: true, @@ -3116,9 +3116,9 @@ export class RouterCore< } } -export class SearchParamError extends Error { } +export class SearchParamError extends Error {} -export class PathParamError extends Error { } +export class PathParamError extends Error {} const normalize = (str: string) => str.endsWith('/') && str.length > 1 ? str.slice(0, -1) : str From 6262e0b520ae6a469029cc4279e3deedc1350dae Mon Sep 17 00:00:00 2001 From: Sheraff Date: Mon, 11 Aug 2025 20:16:43 +0200 Subject: [PATCH 03/13] dont force executeHead to be async --- packages/router-core/src/router.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 674c977319..88c01c6d49 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2463,7 +2463,7 @@ export class RouterCore< let loaderIsRunningAsync = false const route = this.looseRoutesById[routeId]! - const executeHead = async () => { + const executeHead = () => { const match = this.getMatch(matchId) // in case of a redirecting match during preload, the match does not exist if (!match) { @@ -2483,14 +2483,11 @@ export class RouterCore< loaderData: match.loaderData, } - const [headFnContent, scripts, headers] = await Promise.all( - [ + return Promise.all([ route.options.head?.(assetContext), route.options.scripts?.(assetContext), route.options.headers?.(assetContext), - ], - ) - + ]).then(([headFnContent, scripts, headers]) => { const meta = headFnContent?.meta const links = headFnContent?.links const headScripts = headFnContent?.scripts @@ -2504,6 +2501,7 @@ export class RouterCore< scripts, styles, } +}) } const potentialPendingMinPromise = async () => { From a5f1810e2f68546f48a3d7e3f8dd7c84726ab963 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 11 Aug 2025 18:17:34 +0000 Subject: [PATCH 04/13] ci: apply automated fixes --- packages/router-core/src/router.ts | 34 +++++++++++++++--------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 88c01c6d49..33248e9867 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2484,24 +2484,24 @@ export class RouterCore< } return Promise.all([ - route.options.head?.(assetContext), - route.options.scripts?.(assetContext), - route.options.headers?.(assetContext), - ]).then(([headFnContent, scripts, headers]) => { - const meta = headFnContent?.meta - const links = headFnContent?.links - const headScripts = headFnContent?.scripts - const styles = headFnContent?.styles + route.options.head?.(assetContext), + route.options.scripts?.(assetContext), + route.options.headers?.(assetContext), + ]).then(([headFnContent, scripts, headers]) => { + const meta = headFnContent?.meta + const links = headFnContent?.links + const headScripts = headFnContent?.scripts + const styles = headFnContent?.styles - return { - meta, - links, - headScripts, - headers, - scripts, - styles, - } -}) + return { + meta, + links, + headScripts, + headers, + scripts, + styles, + } + }) } const potentialPendingMinPromise = async () => { From f1d0bb3a9b6da3e375906c7af95edd8fd00e3d7f Mon Sep 17 00:00:00 2001 From: Sheraff Date: Mon, 11 Aug 2025 22:48:05 +0200 Subject: [PATCH 05/13] debug: does it work serially? --- packages/router-core/src/router.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 33248e9867..d45bcf9052 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2483,11 +2483,20 @@ export class RouterCore< loaderData: match.loaderData, } - return Promise.all([ - route.options.head?.(assetContext), - route.options.scripts?.(assetContext), - route.options.headers?.(assetContext), - ]).then(([headFnContent, scripts, headers]) => { + // DEBUG: this is just for testing purposes, the goal is to use `Promise.all`, not this weird thing + return Promise.resolve(async () => { + const headFnContent = + await route.options.head?.(assetContext) + const scripts = + await route.options.scripts?.(assetContext) + const headers = + await route.options.headers?.(assetContext) + + // return Promise.all([ + // route.options.head?.(assetContext), + // route.options.scripts?.(assetContext), + // route.options.headers?.(assetContext), + // ]).then(([headFnContent, scripts, headers]) => { const meta = headFnContent?.meta const links = headFnContent?.links const headScripts = headFnContent?.scripts From f4bfcc89aee0ac991e5bbcb6ee786c5b3f7a0c28 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Mon, 11 Aug 2025 23:25:34 +0200 Subject: [PATCH 06/13] Revert "debug: does it work serially?" This reverts commit f1d0bb3a9b6da3e375906c7af95edd8fd00e3d7f. --- packages/router-core/src/router.ts | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index d45bcf9052..33248e9867 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2483,20 +2483,11 @@ export class RouterCore< loaderData: match.loaderData, } - // DEBUG: this is just for testing purposes, the goal is to use `Promise.all`, not this weird thing - return Promise.resolve(async () => { - const headFnContent = - await route.options.head?.(assetContext) - const scripts = - await route.options.scripts?.(assetContext) - const headers = - await route.options.headers?.(assetContext) - - // return Promise.all([ - // route.options.head?.(assetContext), - // route.options.scripts?.(assetContext), - // route.options.headers?.(assetContext), - // ]).then(([headFnContent, scripts, headers]) => { + return Promise.all([ + route.options.head?.(assetContext), + route.options.scripts?.(assetContext), + route.options.headers?.(assetContext), + ]).then(([headFnContent, scripts, headers]) => { const meta = headFnContent?.meta const links = headFnContent?.links const headScripts = headFnContent?.scripts From 0145ed11891aafecde676a93db3f0627834d270d Mon Sep 17 00:00:00 2001 From: Sheraff Date: Tue, 12 Aug 2025 00:45:25 +0200 Subject: [PATCH 07/13] is it actually a forced update issue? --- packages/router-core/src/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 33248e9867..903f051334 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2474,7 +2474,7 @@ export class RouterCore< !route.options.scripts && !route.options.headers ) { - return + return {} } const assetContext = { matches, From 2b5ee8978b3a02e59c499f694d52f1a15f0bd48b Mon Sep 17 00:00:00 2001 From: Sheraff Date: Tue, 12 Aug 2025 00:51:59 +0200 Subject: [PATCH 08/13] Revert "is it actually a forced update issue?" This reverts commit 0145ed11891aafecde676a93db3f0627834d270d. --- packages/router-core/src/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 903f051334..33248e9867 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2474,7 +2474,7 @@ export class RouterCore< !route.options.scripts && !route.options.headers ) { - return {} + return } const assetContext = { matches, From 5bb291b32fd7a0c4daea6f7baa2b7ca543dab0da Mon Sep 17 00:00:00 2001 From: Sheraff Date: Tue, 12 Aug 2025 00:53:34 +0200 Subject: [PATCH 09/13] is it the non reactive update order? --- packages/router-core/src/router.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 33248e9867..b642947b1d 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2687,10 +2687,13 @@ export class RouterCore< const head = await executeHead() if (head) { - updateMatch(matchId, (prev) => ({ - ...prev, - ...head, - })) + updateMatch(matchId, (prev) => { + prev._nonReactive.loaderPromise = undefined + return { + ...prev, + ...head, + } + }) } const match = this.getMatch(matchId)! match._nonReactive.loaderPromise = undefined From 120d3dd5c2781c224ff303abed609b0237ad7dd0 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Tue, 12 Aug 2025 08:14:59 +0200 Subject: [PATCH 10/13] Revert "is it the non reactive update order?" This reverts commit 5bb291b32fd7a0c4daea6f7baa2b7ca543dab0da. --- packages/router-core/src/router.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index b642947b1d..33248e9867 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2687,13 +2687,10 @@ export class RouterCore< const head = await executeHead() if (head) { - updateMatch(matchId, (prev) => { - prev._nonReactive.loaderPromise = undefined - return { - ...prev, - ...head, - } - }) + updateMatch(matchId, (prev) => ({ + ...prev, + ...head, + })) } const match = this.getMatch(matchId)! match._nonReactive.loaderPromise = undefined From b70625978d8e32b3585b338dd55e75d8b83164a4 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Thu, 14 Aug 2025 23:51:34 +0200 Subject: [PATCH 11/13] sometimes match doesnt exist, when redirecting during a preload --- packages/router-core/src/router.ts | 92 +++++++++++++++++------------- 1 file changed, 51 insertions(+), 41 deletions(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 0c5641d98c..3c658ae07f 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1393,8 +1393,8 @@ export class RouterCore< if (!match) return match.abortController.abort() - match._nonReactive.pendingTimeout = undefined clearTimeout(match._nonReactive.pendingTimeout) + match._nonReactive.pendingTimeout = undefined } cancelMatches = () => { @@ -2121,7 +2121,7 @@ export class RouterCore< triggerOnReady() } - const handleRedirectAndNotFound = (match: AnyRouteMatch, err: any) => { + const handleRedirectAndNotFound = (match: AnyRouteMatch | undefined, err: any) => { if (isRedirect(err) || isNotFound(err)) { if (isRedirect(err)) { if (err.redirectHandled) { @@ -2131,27 +2131,30 @@ export class RouterCore< } } - match._nonReactive.beforeLoadPromise?.resolve() - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.beforeLoadPromise = undefined - match._nonReactive.loaderPromise = undefined - - updateMatch(match.id, (prev) => ({ - ...prev, - status: isRedirect(err) - ? 'redirected' - : isNotFound(err) - ? 'notFound' - : 'error', - isFetching: false, - error: err, - })) + // in case of a redirecting match during preload, the match does not exist + if (match) { + match._nonReactive.beforeLoadPromise?.resolve() + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.beforeLoadPromise = undefined + match._nonReactive.loaderPromise = undefined + + updateMatch(match.id, (prev) => ({ + ...prev, + status: isRedirect(err) + ? 'redirected' + : isNotFound(err) + ? 'notFound' + : 'error', + isFetching: false, + error: err, + })) - if (!(err as any).routeId) { - ;(err as any).routeId = match.routeId - } + if (!(err as any).routeId) { + ;(err as any).routeId = match.routeId + } - match._nonReactive.loadPromise?.resolve() + match._nonReactive.loadPromise?.resolve() + } if (isRedirect(err)) { rendered = true @@ -2204,13 +2207,13 @@ export class RouterCore< err.routerCode = routerCode firstBadMatchIndex = firstBadMatchIndex ?? index - handleRedirectAndNotFound(this.getMatch(matchId)!, err) + handleRedirectAndNotFound(this.getMatch(matchId), err) try { route.options.onError?.(err) } catch (errorHandlerErr) { err = errorHandlerErr - handleRedirectAndNotFound(this.getMatch(matchId)!, err) + handleRedirectAndNotFound(this.getMatch(matchId), err) } updateMatch(matchId, (prev) => { @@ -2516,8 +2519,9 @@ export class RouterCore< const prevMatch = this.getMatch(matchId)! if (shouldSkipLoader(matchId)) { if (this.isServer) { - const head = await executeHead() - if (head) { + const headResult = executeHead() + if (headResult) { + const head = await headResult updateMatch(matchId, (prev) => ({ ...prev, ...head, @@ -2634,7 +2638,7 @@ export class RouterCore< await route.options.loader?.(getLoaderContext()) handleRedirectAndNotFound( - this.getMatch(matchId)!, + this.getMatch(matchId), loaderData, ) updateMatch(matchId, (prev) => ({ @@ -2646,7 +2650,8 @@ export class RouterCore< // so we need to wait for it to resolve before // we can use the options await route._lazyPromise - const head = await executeHead() + const headResult = executeHead() + const head = headResult ? await headResult : undefined await potentialPendingMinPromise() // Last but not least, wait for the the components @@ -2665,18 +2670,19 @@ export class RouterCore< await potentialPendingMinPromise() - handleRedirectAndNotFound(this.getMatch(matchId)!, e) + handleRedirectAndNotFound(this.getMatch(matchId), e) try { route.options.onError?.(e) } catch (onErrorError) { error = onErrorError handleRedirectAndNotFound( - this.getMatch(matchId)!, + this.getMatch(matchId), onErrorError, ) } - const head = await executeHead() + const headResult = executeHead() + const head = headResult ? await headResult : undefined updateMatch(matchId, (prev) => ({ ...prev, error, @@ -2686,16 +2692,19 @@ export class RouterCore< })) } } catch (err) { - const head = await executeHead() - - if (head) { - updateMatch(matchId, (prev) => ({ - ...prev, - ...head, - })) + const match = this.getMatch(matchId) + // in case of a redirecting match during preload, the match does not exist + if (match) { + const headResult = executeHead() + if (headResult) { + const head = await headResult + updateMatch(matchId, (prev) => ({ + ...prev, + ...head, + })) + } + match._nonReactive.loaderPromise = undefined } - const match = this.getMatch(matchId)! - match._nonReactive.loaderPromise = undefined handleRedirectAndNotFound(match, err) } } @@ -2731,8 +2740,9 @@ export class RouterCore< // 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 head = await executeHead() - if (head) { + const headResult = executeHead() + if (headResult) { + const head = await headResult updateMatch(matchId, (prev) => ({ ...prev, ...head, From 78802ef6d10bfb2819285b555480dcc5ee5cc493 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 14 Aug 2025 21:52:32 +0000 Subject: [PATCH 12/13] ci: apply automated fixes --- packages/router-core/src/router.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 3c658ae07f..795aa0a195 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2121,7 +2121,10 @@ export class RouterCore< triggerOnReady() } - const handleRedirectAndNotFound = (match: AnyRouteMatch | undefined, err: any) => { + const handleRedirectAndNotFound = ( + match: AnyRouteMatch | undefined, + err: any, + ) => { if (isRedirect(err) || isNotFound(err)) { if (isRedirect(err)) { if (err.redirectHandled) { From 4882c465b0a603c36ff5d732f6ca30039de059f4 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Fri, 15 Aug 2025 09:34:45 +0200 Subject: [PATCH 13/13] redirection in preload from 11 to 8 updates --- .../store-updates-during-navigation.test.tsx | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/react-router/tests/store-updates-during-navigation.test.tsx b/packages/react-router/tests/store-updates-during-navigation.test.tsx index 312b9c9fdd..33e3443fdc 100644 --- a/packages/react-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/react-router/tests/store-updates-during-navigation.test.tsx @@ -16,7 +16,7 @@ afterEach(() => { cleanup() }) -async function setupAndRun({ +function setup({ beforeLoad, loader, head, @@ -78,6 +78,10 @@ async function setupAndRun({ render() + return { select, router } +} + +async function run({ select }: ReturnType, opts?: {}) { // navigate to /posts const link = await waitFor(() => screen.getByRole('link', { name: 'Posts' })) const before = select.mock.calls.length @@ -93,7 +97,7 @@ async function setupAndRun({ describe("Store doesn't update *too many* times during navigation", () => { test('async loader, async beforeLoad, pendingMs', async () => { - const updates = await setupAndRun({ + const params = setup({ beforeLoad: () => new Promise((resolve) => setTimeout(resolve, 100)), loader: () => new Promise((resolve) => setTimeout(resolve, 100)), @@ -101,22 +105,29 @@ describe("Store doesn't update *too many* times during navigation", () => { defaultPendingMinMs: 300, }) + const updates = await run(params) + // 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) }) - test('redirection', async () => { - const updates = await setupAndRun({ - beforeLoad: () => { + test('redirection in preload', async () => { + const { select, router } = setup({ + loader: () => { throw redirect({ to: '/other' }) }, }) + const before = select.mock.calls.length + await router.preloadRoute({ to: '/posts' }) + const after = select.mock.calls.length + const updates = after - before + // 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(26) + expect(updates).toBe(8) }) })