From 393b5247985401955f17fcc8946a100c86f05309 Mon Sep 17 00:00:00 2001 From: Paul Sachs Date: Thu, 30 Oct 2025 11:19:41 -0400 Subject: [PATCH] Merge __beforeLoadContext for pending loading state --- .../react-router/tests/routeContext.test.tsx | 172 ++++++++++++++++++ packages/router-core/src/load-matches.ts | 22 ++- 2 files changed, 191 insertions(+), 3 deletions(-) diff --git a/packages/react-router/tests/routeContext.test.tsx b/packages/react-router/tests/routeContext.test.tsx index eef424c867f..e1ea5113e72 100644 --- a/packages/react-router/tests/routeContext.test.tsx +++ b/packages/react-router/tests/routeContext.test.tsx @@ -3128,4 +3128,176 @@ describe('useRouteContext in the component', () => { expect(content).toBeInTheDocument() }) + + test("reproducer #4998 - on navigate (with preload), route component isn't rendered with undefined context if beforeLoad is pending", async () => { + const beforeLoad = vi.fn() + const select = vi.fn() + let resolved = 0 + const rootRoute = createRootRoute() + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () => To foo, + }) + const fooRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + beforeLoad: async (...args) => { + beforeLoad(...args) + await sleep(WAIT_TIME) + resolved += 1 + return { foo: resolved } + }, + component: () => { + fooRoute.useRouteContext({ select }) + return

Foo index page

+ }, + pendingComponent: () => 'loading', + }) + const routeTree = rootRoute.addChildren([indexRoute, fooRoute]) + const router = createRouter({ + routeTree, + history, + defaultPreload: 'intent', + defaultPendingMs: 0, + }) + + render() + const linkToFoo = await screen.findByText('To foo') + + fireEvent.focus(linkToFoo) + expect(beforeLoad).toHaveBeenCalledTimes(1) + expect(resolved).toBe(0) + + await sleep(WAIT_TIME) + + expect(beforeLoad).toHaveBeenCalledTimes(1) + expect(resolved).toBe(1) + expect(beforeLoad).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + cause: 'preload', + preload: true, + }), + ) + + expect(select).not.toHaveBeenCalled() + + fireEvent.click(linkToFoo) + expect(beforeLoad).toHaveBeenCalledTimes(2) + expect(resolved).toBe(1) + + await screen.findByText('Foo index page') + + expect(beforeLoad).toHaveBeenCalledTimes(2) + expect(beforeLoad).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + cause: 'enter', + preload: false, + }), + ) + expect(select).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + foo: 1, + }), + ) + expect(select).not.toHaveBeenCalledWith({}) + + await sleep(WAIT_TIME) + expect(beforeLoad).toHaveBeenCalledTimes(2) + expect(resolved).toBe(2) + + // ensure the context has been updated once the beforeLoad has resolved + expect(select).toHaveBeenLastCalledWith( + expect.objectContaining({ + foo: 2, + }), + ) + + // the route component will be rendered multiple times, ensure it always has the context + expect(select).not.toHaveBeenCalledWith({}) + }) + + test('beforeLoad receives fresh context on second run, not stale preloaded context', async () => { + const beforeLoadContextSpy = vi.fn() + let resolved = 0 + const rootRoute = createRootRoute({ + beforeLoad: () => { + return { rootValue: 'root' } + }, + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () => To foo, + }) + const fooRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + beforeLoad: async ({ context }) => { + // Capture what context beforeLoad receives + beforeLoadContextSpy(context) + await sleep(WAIT_TIME) + resolved += 1 + return { foo: resolved } + }, + component: () =>

Foo page

, + pendingComponent: () => 'loading', + }) + const routeTree = rootRoute.addChildren([indexRoute, fooRoute]) + const router = createRouter({ + routeTree, + history, + defaultPreload: 'intent', + defaultPendingMs: 0, + }) + + render() + const linkToFoo = await screen.findByText('To foo') + + // Preload foo by hovering + fireEvent.focus(linkToFoo) + await sleep(WAIT_TIME) + + expect(resolved).toBe(1) + // First call during preload - should receive parent context only + expect(beforeLoadContextSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + rootValue: 'root', + }), + ) + // Should NOT have foo in context yet + expect(beforeLoadContextSpy).toHaveBeenNthCalledWith( + 1, + expect.not.objectContaining({ + foo: expect.anything(), + }), + ) + + beforeLoadContextSpy.mockClear() + + // Navigate to foo + fireEvent.click(linkToFoo) + await screen.findByText('Foo page') + await sleep(WAIT_TIME) + + expect(resolved).toBe(2) + // Second call during navigation - should receive parent context only, NOT the preloaded foo context + expect(beforeLoadContextSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + rootValue: 'root', + }), + ) + // Should NOT receive the previous foo value from preload + expect(beforeLoadContextSpy).toHaveBeenNthCalledWith( + 1, + expect.not.objectContaining({ + foo: expect.anything(), + }), + ) + }) }) diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 6889d886773..e7134efa3c0 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -369,7 +369,23 @@ const executeBeforeLoad = ( const parentMatchContext = parentMatch?.context ?? inner.router.options.context ?? undefined - const context = { ...parentMatchContext, ...match.__routeContext } + // Context for beforeLoad function - always fresh, never includes previous __beforeLoadContext + const beforeLoadFnInputContext = { + ...parentMatchContext, + ...match.__routeContext, + } + + // Context for component during pending - includes previous __beforeLoadContext if match was successful + // This preserves context from preloads/previous navigations during the pending state, + // but only for the component, not for beforeLoad itself + const pendingContext = + match.__beforeLoadContext && match.status === 'success' + ? { + ...match.__beforeLoadContext, + ...parentMatchContext, + ...match.__routeContext, + } + : beforeLoadFnInputContext let isPending = false const pending = () => { @@ -380,7 +396,7 @@ const executeBeforeLoad = ( isFetching: 'beforeLoad', fetchCount: prev.fetchCount + 1, abortController, - context, + context: pendingContext, })) } @@ -420,7 +436,7 @@ const executeBeforeLoad = ( abortController, params, preload, - context, + context: beforeLoadFnInputContext, location: inner.location, navigate: (opts: any) => inner.router.navigate({