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 4c18a56815..5fe748c0df 100644 --- a/packages/react-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/react-router/tests/store-updates-during-navigation.test.tsx @@ -73,6 +73,6 @@ describe('Store updates during navigation', () => { // 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(after - before).toBe(19) + expect(after - before).toBe(14) }) }) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 58b996d866..54b8cae28d 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2299,6 +2299,20 @@ export class RouterCore< continue } + const handleSearchAndParamSerialErrors = ( + index: number, + matchId: string, + ) => { + const { paramsError, searchError } = this.getMatch(matchId)! + if (paramsError) { + handleSerialError(index, paramsError, 'PARSE_PARAMS') + } + + if (searchError) { + handleSerialError(index, searchError, 'VALIDATE_SEARCH') + } + } + const shouldPending = !!( onReady && !this.isServer && @@ -2311,8 +2325,6 @@ export class RouterCore< (route.options.pendingComponent ?? (this.options as any)?.defaultPendingComponent) ) - - let executeBeforeLoad = true const setupPendingTimeout = () => { const match = this.getMatch(matchId)! if ( @@ -2329,14 +2341,35 @@ export class RouterCore< match._nonReactive.pendingTimeout = pendingTimeout } } - if ( - // If we are in the middle of a load, either of these will be present - // (not to be confused with `loadPromise`, which is always defined) + + // If we are in the middle of a load, either of these will be present + // (not to be confused with `loadPromise`, which is always defined) + const hasBeforeLoadOrLoaderPromise = existingMatch._nonReactive.beforeLoadPromise || existingMatch._nonReactive.loaderPromise - ) { + + if (hasBeforeLoadOrLoaderPromise) { setupPendingTimeout() + } + + // we can bail out early if there is no `beforeLoad` + if (!route.options.beforeLoad) { + handleSearchAndParamSerialErrors(index, matchId) + const parentMatchContext = + parentMatch?.context ?? this.options.context ?? {} + updateMatch(matchId, (prev) => ({ + ...prev, + context: { + ...parentMatchContext, + ...prev.__routeContext, + }, + })) + continue + } + + let executeBeforeLoad = true + if (hasBeforeLoadOrLoaderPromise) { // Wait for the beforeLoad to resolve before we continue await existingMatch._nonReactive.beforeLoadPromise const match = this.getMatch(matchId)! @@ -2349,6 +2382,10 @@ export class RouterCore< handleRedirectAndNotFound(match, match.error) } } + + let beforeLoadContext = + this.getMatch(matchId)!.__beforeLoadContext + if (executeBeforeLoad) { // If we are not in the middle of a load OR the previous load failed, start it try { @@ -2362,15 +2399,7 @@ export class RouterCore< prevLoadPromise?.resolve() }) - const { paramsError, searchError } = this.getMatch(matchId)! - - if (paramsError) { - handleSerialError(index, paramsError, 'PARSE_PARAMS') - } - - if (searchError) { - handleSerialError(index, searchError, 'VALIDATE_SEARCH') - } + handleSearchAndParamSerialErrors(index, matchId) setupPendingTimeout() @@ -2415,8 +2444,8 @@ export class RouterCore< matches, } - const beforeLoadContext = - await route.options.beforeLoad?.(beforeLoadFnContext) + beforeLoadContext = + await route.options.beforeLoad!(beforeLoadFnContext) if ( isRedirect(beforeLoadContext) || @@ -2424,20 +2453,15 @@ export class RouterCore< ) { handleSerialError(index, beforeLoadContext, 'BEFORE_LOAD') } - - updateMatch(matchId, (prev) => { - return { - ...prev, - __beforeLoadContext: beforeLoadContext, - context: { - ...parentMatchContext, - ...prev.__routeContext, - ...beforeLoadContext, - }, - abortController, - } - }) } catch (err) { + updateMatch(matchId, (prev) => ({ + ...prev, + __beforeLoadContext: beforeLoadContext, + context: { + ...prev.context, + ...beforeLoadContext, + }, + })) handleSerialError(index, err, 'BEFORE_LOAD') } @@ -2447,7 +2471,12 @@ export class RouterCore< return { ...prev, + __beforeLoadContext: beforeLoadContext, isFetching: false, + context: { + ...prev.context, + ...beforeLoadContext, + }, } }) } @@ -2469,21 +2498,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, @@ -2494,21 +2535,21 @@ export class RouterCore< } } - const potentialPendingMinPromise = async () => { + const potentialPendingMinPromise = () => { const latestMatch = this.getMatch(matchId)! - if (latestMatch._nonReactive.minPendingPromise) { - await latestMatch._nonReactive.minPendingPromise - } + return latestMatch._nonReactive.minPendingPromise } const prevMatch = this.getMatch(matchId)! 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)! } } @@ -2604,29 +2645,30 @@ export class RouterCore< try { if ( !this.isServer || - (this.isServer && - this.getMatch(matchId)!.ssr === true) + this.getMatch(matchId)!.ssr === true ) { this.loadRouteChunk(route) } - updateMatch(matchId, (prev) => ({ - ...prev, - isFetching: 'loader', - })) + if (route.options.loader) { + updateMatch(matchId, (prev) => ({ + ...prev, + isFetching: 'loader', + })) - // Kick off the loader! - const loaderData = - await route.options.loader?.(getLoaderContext()) + // Kick off the loader! + const loaderData = + await route.options.loader(getLoaderContext()) - handleRedirectAndNotFound( - this.getMatch(matchId)!, - loaderData, - ) - updateMatch(matchId, (prev) => ({ - ...prev, - loaderData, - })) + handleRedirectAndNotFound( + this.getMatch(matchId)!, + loaderData, + ) + updateMatch(matchId, (prev) => ({ + ...prev, + loaderData, + })) + } // Lazy option can modify the route options, // so we need to wait for it to resolve before @@ -2674,14 +2716,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) } } @@ -2717,10 +2760,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) { diff --git a/packages/solid-router/tests/Transitioner.test.tsx b/packages/solid-router/tests/Transitioner.test.tsx index ffd3ffb100..dffe2bd4fd 100644 --- a/packages/solid-router/tests/Transitioner.test.tsx +++ b/packages/solid-router/tests/Transitioner.test.tsx @@ -30,13 +30,12 @@ describe('Transitioner', () => { // Mock router.load() to verify it gets called const loadSpy = vi.spyOn(router, 'load') - await router.load() - render(() => ) + await router.latestLoadPromise // Wait for the createRenderEffect to run and call router.load() await waitFor(() => { - expect(loadSpy).toHaveBeenCalledTimes(2) + expect(loadSpy).toHaveBeenCalledTimes(1) expect(loader).toHaveBeenCalledTimes(1) })