From b1c56315045813db9c0634e5fb0e033d53fbf70b Mon Sep 17 00:00:00 2001 From: Jakob Norlin Date: Tue, 9 Dec 2025 18:52:22 +0100 Subject: [PATCH 1/4] test for route context being undefined when navigating to a new route faster than beforeLoad resolves --- .../react-router/tests/routeContext.test.tsx | 56 ++++++++++++++++++ .../solid-router/tests/routeContext.test.tsx | 59 +++++++++++++++++++ 2 files changed, 115 insertions(+) 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/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 () => { From 33d9724a30a50377e8ea0d04a02d553903e169da Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Tue, 9 Dec 2025 18:52:22 +0100 Subject: [PATCH 2/4] fix: commit route context --- .../store-updates-during-navigation.test.tsx | 20 ++++----- packages/router-core/src/load-matches.ts | 44 +++++++++++++++---- 2 files changed, 46 insertions(+), 18 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 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..5032a1e2fe8 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -371,7 +371,6 @@ const executeBeforeLoad = ( const parentMatchContext = parentMatch?.context ?? inner.router.options.context ?? undefined - const context = { ...parentMatchContext, ...match.__routeContext } let isPending = false const pending = () => { @@ -382,7 +381,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 +396,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 +424,8 @@ 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 +453,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() }) @@ -742,6 +743,25 @@ const loadRouteMatch = async ( let loaderIsRunningAsync = false const route = inner.router.looseRoutesById[routeId]! + // Helper to compute and commit context after loader completes + const commitContext = () => { + const parentMatchId = inner.matches[index - 1]?.id + const parentMatch = parentMatchId + ? inner.router.getMatch(parentMatchId)! + : undefined + const parentContext = + parentMatch?.context ?? inner.router.options.context ?? undefined + + inner.updateMatch(matchId, (prev) => ({ + ...prev, + context: { + ...parentContext, + ...prev.__routeContext, + ...prev.__beforeLoadContext, + }, + })) + } + if (shouldSkipLoader(inner, matchId)) { if (inner.router.isServer) { const headResult = executeHead(inner, matchId, route) @@ -816,6 +836,7 @@ const loadRouteMatch = async ( ;(async () => { try { await runLoader(inner, matchId, index, route) + commitContext() const match = inner.router.getMatch(matchId)! match._nonReactive.loaderPromise?.resolve() match._nonReactive.loadPromise?.resolve() @@ -853,6 +874,13 @@ const loadRouteMatch = async ( match._nonReactive.pendingTimeout = undefined if (!loaderIsRunningAsync) match._nonReactive.loaderPromise = undefined match._nonReactive.dehydrated = undefined + + // Commit context now that loader has completed (or was skipped) + // For async loaders, this was already done in the async callback + if (!loaderIsRunningAsync) { + commitContext() + } + const nextIsFetching = loaderIsRunningAsync ? match.isFetching : false if (nextIsFetching !== match.isFetching || match.invalid !== false) { inner.updateMatch(matchId, (prev) => ({ From 131445cafb6d93f0a716644be636426454f48b8f Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Tue, 9 Dec 2025 20:53:24 +0100 Subject: [PATCH 3/4] fix test expectations --- .../store-updates-during-navigation.test.tsx | 5 +++-- .../store-updates-during-navigation.test.tsx | 19 ++++++++++--------- .../store-updates-during-navigation.test.tsx | 18 +++++++++--------- 3 files changed, 22 insertions(+), 20 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 b9ae3922b40..f111be4a038 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,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).toBe(12) + expect(updates).toBeGreaterThanOrEqual(11) // WARN: this is flaky, and sometimes (rarely) is 12 + expect(updates).toBeLessThanOrEqual(13) }) test('redirection in preload', async () => { @@ -170,7 +171,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(11) }) test('nothing', async () => { diff --git a/packages/solid-router/tests/store-updates-during-navigation.test.tsx b/packages/solid-router/tests/store-updates-during-navigation.test.tsx index c69643e1c3d..72c989dcbd0 100644 --- a/packages/solid-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/solid-router/tests/store-updates-during-navigation.test.tsx @@ -136,8 +136,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. - // Note: Solid has different update counts than React due to different reactivity - expect(updates).toBe(10) + expect(updates).toBeGreaterThanOrEqual(11) // WARN: this is flaky, and sometimes (rarely) is 12 + expect(updates).toBeLessThanOrEqual(13) }) test('redirection in preload', async () => { @@ -156,7 +156,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has different update counts than React due to different reactivity - expect(updates).toBe(6) + expect(updates).toBe(7) }) test('sync beforeLoad', async () => { @@ -173,7 +173,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has different update counts than React due to different reactivity - expect(updates).toBe(8) + expect(updates).toBe(10) }) test('nothing', async () => { @@ -226,7 +226,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 () => { @@ -242,7 +242,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).toBe(9) + expect(updates).toBeGreaterThanOrEqual(11) // WARN: this is flaky, and sometimes (rarely) is 12 + expect(updates).toBeLessThanOrEqual(13) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -259,7 +260,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has one fewer update than React due to different reactivity - expect(updates).toBe(7) + expect(updates).toBe(9) }) test('navigate, w/ previous navigation & async loader', async () => { @@ -275,7 +276,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 () => { @@ -293,6 +294,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(2) + expect(updates).toBe(3) }) }) diff --git a/packages/vue-router/tests/store-updates-during-navigation.test.tsx b/packages/vue-router/tests/store-updates-during-navigation.test.tsx index 466a49c545d..dceeeb9295e 100644 --- a/packages/vue-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/vue-router/tests/store-updates-during-navigation.test.tsx @@ -138,7 +138,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(27) + expect(updates).toBe(33) }) test('redirection in preload', async () => { @@ -157,7 +157,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(10) + expect(updates).toBe(13) }) test('sync beforeLoad', async () => { @@ -174,7 +174,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(25) + expect(updates).toBe(31) }) test('nothing', async () => { @@ -188,7 +188,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // Note: Vue has different update counts than React/Solid due to different reactivity // Vue's reactivity model may cause slightly more updates due to computed refs expect(updates).toBeGreaterThanOrEqual(14) // WARN: this is flaky - expect(updates).toBeLessThanOrEqual(20) + expect(updates).toBeLessThanOrEqual(23) }) test('not found in beforeLoad', async () => { @@ -231,7 +231,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(38) + expect(updates).toBe(47) }) test('navigate, w/ preloaded & async loaders', async () => { @@ -248,7 +248,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(18) + expect(updates).toBe(24) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -265,7 +265,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(16) + expect(updates).toBe(22) }) test('navigate, w/ previous navigation & async loader', async () => { @@ -282,7 +282,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(12) + expect(updates).toBe(18) }) test('preload a preloaded route w/ async loader', async () => { @@ -301,6 +301,6 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(4) + expect(updates).toBe(7) }) }) From 562ad189a66105b8801ce062dfc563c8a23f810a Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Tue, 9 Dec 2025 20:53:50 +0100 Subject: [PATCH 4/4] fix child context aggregation --- packages/router-core/src/load-matches.ts | 29 ++++++++++++------------ 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 5032a1e2fe8..3b703d44354 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -371,7 +371,6 @@ const executeBeforeLoad = ( const parentMatchContext = parentMatch?.context ?? inner.router.options.context ?? undefined - let isPending = false const pending = () => { if (isPending) return @@ -425,7 +424,11 @@ const executeBeforeLoad = ( params, preload, // Include parent's __beforeLoadContext so child routes can access it during their beforeLoad - context: { ...parentMatchContext, ...parentMatch?.__beforeLoadContext, ...match.__routeContext }, + context: { + ...parentMatchContext, + ...parentMatch?.__beforeLoadContext, + ...match.__routeContext, + }, location: inner.location, navigate: (opts: any) => inner.router.navigate({ @@ -743,22 +746,20 @@ const loadRouteMatch = async ( let loaderIsRunningAsync = false const route = inner.router.looseRoutesById[routeId]! - // Helper to compute and commit context after loader completes const commitContext = () => { - const parentMatchId = inner.matches[index - 1]?.id - const parentMatch = parentMatchId - ? inner.router.getMatch(parentMatchId)! - : undefined - const parentContext = - parentMatch?.context ?? inner.router.options.context ?? undefined + const context = { ...inner.router.options.context } + + for (let i = 0; i <= index; i++) { + const innerMatch = inner.matches[i] + if (!innerMatch) continue + const m = inner.router.getMatch(innerMatch.id) + if (!m) continue + Object.assign(context, m.__routeContext, m.__beforeLoadContext) + } inner.updateMatch(matchId, (prev) => ({ ...prev, - context: { - ...parentContext, - ...prev.__routeContext, - ...prev.__beforeLoadContext, - }, + context, })) }