diff --git a/packages/react-router/tests/routeContext.test.tsx b/packages/react-router/tests/routeContext.test.tsx index eef424c867f..e42b10d6035 100644 --- a/packages/react-router/tests/routeContext.test.tsx +++ b/packages/react-router/tests/routeContext.test.tsx @@ -9,6 +9,7 @@ import { import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { z } from 'zod' +import { useEffect } from 'react' import { Link, Outlet, @@ -2475,6 +2476,61 @@ describe('useRouteContext in the component', () => { expect(content).toBeInTheDocument() }) + test('route context, (sleep in beforeLoad), with immediate navigation', async () => { + const contextValues: Array<{ data: string }> = [] + + const rootRoute = createRootRoute({ + beforeLoad: async () => { + await sleep(WAIT_TIME) + return { data: 'context-from-beforeLoad' } + }, + component: () => { + const context: { data: string } = rootRoute.useRouteContext() + + // Track all context values we receive + contextValues.push(context) + + return + }, + }) + + function Component() { + const navigate = indexRoute.useNavigate() + + // Navigate away immediately on mount + useEffect(() => { + navigate({ to: '/other' }) + }, [navigate]) + + return
Index page
+ } + + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: Component, + }) + + const otherRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/other', + component: () =>
Other page
, + }) + + const routeTree = rootRoute.addChildren([indexRoute, otherRoute]) + const router = createRouter({ routeTree, history }) + + render() + + // Wait for navigation to complete + await screen.findByText('Other page') + + const allContextsValid = contextValues.every( + (c) => c.data === 'context-from-beforeLoad', + ) + expect(allContextsValid).toBe(true) + }) + test('route context (sleep in loader), present in the index route', async () => { const rootRoute = createRootRoute({}) const indexRoute = createRoute({ 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 fcbc841bddf..b9ae3922b40 100644 --- a/packages/react-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/react-router/tests/store-updates-during-navigation.test.tsx @@ -136,7 +136,7 @@ describe("Store doesn't update *too many* times 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(updates).toBe(10) + expect(updates).toBe(12) }) test('redirection in preload', async () => { @@ -154,7 +154,7 @@ describe("Store doesn't update *too many* times 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(updates).toBe(4) + expect(updates).toBe(5) }) test('sync beforeLoad', async () => { @@ -170,7 +170,7 @@ describe("Store doesn't update *too many* times 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(updates).toBe(9) + expect(updates).toBe(10) }) test('nothing', async () => { @@ -181,8 +181,8 @@ describe("Store doesn't update *too many* times 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(updates).toBeGreaterThanOrEqual(6) // WARN: this is flaky, and sometimes (rarely) is 7 - expect(updates).toBeLessThanOrEqual(7) + expect(updates).toBeGreaterThanOrEqual(8) // WARN: this is flaky, and sometimes (rarely) is 9 + expect(updates).toBeLessThanOrEqual(9) }) test('not found in beforeLoad', async () => { @@ -223,7 +223,7 @@ describe("Store doesn't update *too many* times 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(updates).toBe(16) + expect(updates).toBe(19) }) test('navigate, w/ preloaded & async loaders', async () => { @@ -239,7 +239,7 @@ describe("Store doesn't update *too many* times 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(updates).toBe(7) + expect(updates).toBe(9) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -255,7 +255,7 @@ describe("Store doesn't update *too many* times 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(updates).toBe(6) + expect(updates).toBe(8) }) test('navigate, w/ previous navigation & async loader', async () => { @@ -271,7 +271,7 @@ describe("Store doesn't update *too many* times 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(updates).toBe(5) + expect(updates).toBe(7) }) test('preload a preloaded route w/ async loader', async () => { @@ -289,6 +289,6 @@ describe("Store doesn't update *too many* times 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(updates).toBe(1) + expect(updates).toBe(2) }) }) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 234be73df95..21150ae3502 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -371,8 +371,6 @@ const executeBeforeLoad = ( const parentMatchContext = parentMatch?.context ?? inner.router.options.context ?? undefined - const context = { ...parentMatchContext, ...match.__routeContext } - let isPending = false const pending = () => { if (isPending) return @@ -382,7 +380,9 @@ const executeBeforeLoad = ( isFetching: 'beforeLoad', fetchCount: prev.fetchCount + 1, abortController, - context, + // Note: We intentionally don't update context here. + // Context should only be updated after beforeLoad resolves to avoid + // components seeing incomplete context during async beforeLoad execution. })) } @@ -395,7 +395,8 @@ const executeBeforeLoad = ( })) } - // if there is no `beforeLoad` option, skip everything, batch update the store, return early + // if there is no `beforeLoad` option, just mark as pending and resolve + // Context will be updated later in loadRouteMatch after loader completes if (!route.options.beforeLoad) { batch(() => { pending() @@ -422,7 +423,12 @@ const executeBeforeLoad = ( abortController, params, preload, - context, + // Include parent's __beforeLoadContext so child routes can access it during their beforeLoad + context: { + ...parentMatchContext, + ...parentMatch?.__beforeLoadContext, + ...match.__routeContext, + }, location: inner.location, navigate: (opts: any) => inner.router.navigate({ @@ -450,13 +456,11 @@ const executeBeforeLoad = ( batch(() => { pending() + // Only store __beforeLoadContext here, don't update context yet + // Context will be updated in loadRouteMatch after loader completes inner.updateMatch(matchId, (prev) => ({ ...prev, __beforeLoadContext: beforeLoadContext, - context: { - ...prev.context, - ...beforeLoadContext, - }, })) resolve() }) @@ -605,6 +609,7 @@ const runLoader = async ( matchId: string, index: number, route: AnyRoute, + contextUpdater: (prev: AnyRouteMatch) => any, ): Promise => { try { // If the Matches component rendered @@ -681,6 +686,7 @@ const runLoader = async ( status: 'success', isFetching: false, updatedAt: Date.now(), + context: contextUpdater(prev), ...head, })) } catch (e) { @@ -742,6 +748,23 @@ const loadRouteMatch = async ( let loaderIsRunningAsync = false const route = inner.router.looseRoutesById[routeId]! + // Helper to compute and commit context after loader completes + let updatedContext = false + const contextUpdater = (prev: AnyRouteMatch) => { + const parentMatchId = inner.matches[index - 1]?.id + const parentMatch = parentMatchId + ? inner.router.getMatch(parentMatchId)! + : undefined + const parentContext = + parentMatch?.context ?? inner.router.options.context ?? undefined + updatedContext = true + return { + ...parentContext, + ...prev.__routeContext, + ...prev.__beforeLoadContext, + } + } + if (shouldSkipLoader(inner, matchId)) { if (inner.router.isServer) { const headResult = executeHead(inner, matchId, route) @@ -815,7 +838,7 @@ const loadRouteMatch = async ( loaderIsRunningAsync = true ;(async () => { try { - await runLoader(inner, matchId, index, route) + await runLoader(inner, matchId, index, route, contextUpdater) const match = inner.router.getMatch(matchId)! match._nonReactive.loaderPromise?.resolve() match._nonReactive.loadPromise?.resolve() @@ -827,7 +850,7 @@ const loadRouteMatch = async ( } })() } else if (status !== 'success' || (loaderShouldRunAsync && inner.sync)) { - await runLoader(inner, matchId, index, route) + await runLoader(inner, matchId, index, route, contextUpdater) } else { // if the loader did not run, still update head. // reason: parent's beforeLoad may have changed the route context @@ -838,6 +861,7 @@ const loadRouteMatch = async ( inner.updateMatch(matchId, (prev) => ({ ...prev, ...head, + context: contextUpdater(prev), })) } } @@ -859,11 +883,21 @@ const loadRouteMatch = async ( ...prev, isFetching: nextIsFetching, invalid: false, + context: updatedContext ? prev.context : contextUpdater(prev), })) return inner.router.getMatch(matchId)! - } else { - return match } + + if (!updatedContext) { + // Commit context now that loader has completed (or was skipped) + // For async loaders, this was already done in the async callback + inner.updateMatch(matchId, (prev) => ({ + ...prev, + context: contextUpdater(prev), + })) + } + + return match } export async function loadMatches(arg: { diff --git a/packages/solid-router/tests/routeContext.test.tsx b/packages/solid-router/tests/routeContext.test.tsx index b83022f7dc4..b7e0a902a91 100644 --- a/packages/solid-router/tests/routeContext.test.tsx +++ b/packages/solid-router/tests/routeContext.test.tsx @@ -2,6 +2,7 @@ import { cleanup, fireEvent, render, screen } from '@solidjs/testing-library' import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { z } from 'zod' +import { createEffect, onMount } from 'solid-js' import { Link, Outlet, @@ -2392,6 +2393,64 @@ describe('useRouteContext in the component', () => { expect(content).toBeInTheDocument() }) + // Note: This test passes in solid-router but fails in react-router, even + // though in issue (GitHub #6040), the bug manifests in both routers. + test('route context, (sleep in beforeLoad), with immediate navigation', async () => { + const contextValues: Array<{ data: string }> = [] + + const rootRoute = createRootRoute({ + beforeLoad: async () => { + await sleep(WAIT_TIME) + return { data: 'context-from-beforeLoad' } + }, + shellComponent: () => { + const context = rootRoute.useRouteContext() + + // Track context value at render time + createEffect(() => { + const contextValue: { data: string } = context() + contextValues.push(contextValue) + }) + + return + }, + }) + + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () => { + const navigate = indexRoute.useNavigate() + + // Navigate away immediately on mount + onMount(() => { + navigate({ to: '/other' }) + }) + + return
Index page
+ }, + }) + + const otherRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/other', + component: () =>
Other page
, + }) + + const routeTree = rootRoute.addChildren([indexRoute, otherRoute]) + const router = createRouter({ routeTree, history }) + + render(() => ) + + // Wait for navigation to complete + await screen.findByText('Other page') + + const allContextsValid = contextValues.every( + (c) => c.data === 'context-from-beforeLoad', + ) + expect(allContextsValid).toBe(true) + }) + test('route context (sleep in loader), present root route', async () => { const rootRoute = createRootRoute({ loader: async () => {