diff --git a/.changeset/fair-weeks-beam.md b/.changeset/fair-weeks-beam.md new file mode 100644 index 0000000000..91058cfe09 --- /dev/null +++ b/.changeset/fair-weeks-beam.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix single fetch bug where no revalidation request would be made when navigating upwards to a reused parent route diff --git a/packages/react-router/__tests__/router/data-strategy-test.ts b/packages/react-router/__tests__/router/data-strategy-test.ts index cc6aed6640..24b4f534bc 100644 --- a/packages/react-router/__tests__/router/data-strategy-test.ts +++ b/packages/react-router/__tests__/router/data-strategy-test.ts @@ -568,6 +568,92 @@ describe("router dataStrategy", () => { child: "CHILD", }); }); + + it("does not short circuit when there are no matchesToLoad", async () => { + let dataStrategy = mockDataStrategy(async ({ matches }) => { + let results = await Promise.all( + matches.map((m) => m.resolve((handler) => handler())) + ); + // Don't use keyedResults since it checks for shouldLoad and this test + // is always loading + return results.reduce( + (acc, r, i) => Object.assign(acc, { [matches[i].route.id]: r }), + {} + ); + }); + let t = setup({ + routes: [ + { + path: "/", + }, + { + id: "parent", + path: "/parent", + loader: true, + children: [ + { + id: "child", + path: "child", + loader: true, + }, + ], + }, + ], + dataStrategy, + }); + + let A = await t.navigate("/parent"); + await A.loaders.parent.resolve("PARENT1"); + expect(A.loaders.parent.stub).toHaveBeenCalled(); + expect(t.router.state.loaderData).toEqual({ + parent: "PARENT1", + }); + expect(dataStrategy.mock.calls[0][0].matches).toEqual([ + expect.objectContaining({ + route: expect.objectContaining({ + id: "parent", + }), + }), + ]); + + let B = await t.navigate("/parent/child"); + await B.loaders.parent.resolve("PARENT2"); + await B.loaders.child.resolve("CHILD"); + expect(B.loaders.parent.stub).toHaveBeenCalled(); + expect(B.loaders.child.stub).toHaveBeenCalled(); + expect(t.router.state.loaderData).toEqual({ + parent: "PARENT2", + child: "CHILD", + }); + expect(dataStrategy.mock.calls[1][0].matches).toEqual([ + expect.objectContaining({ + route: expect.objectContaining({ + id: "parent", + }), + }), + expect.objectContaining({ + route: expect.objectContaining({ + id: "child", + }), + }), + ]); + + let C = await t.navigate("/parent"); + await C.loaders.parent.resolve("PARENT3"); + expect(C.loaders.parent.stub).toHaveBeenCalled(); + expect(t.router.state.loaderData).toEqual({ + parent: "PARENT3", + }); + expect(dataStrategy.mock.calls[2][0].matches).toEqual([ + expect.objectContaining({ + route: expect.objectContaining({ + id: "parent", + }), + }), + ]); + + expect(dataStrategy).toHaveBeenCalledTimes(3); + }); }); describe("actions", () => { diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 84f747578e..158dc2639f 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -1927,8 +1927,13 @@ export function createRouter(init: RouterInit): Router { pendingNavigationLoadId = ++incrementingLoadId; - // Short circuit if we have no loaders to run - if (matchesToLoad.length === 0 && revalidatingFetchers.length === 0) { + // Short circuit if we have no loaders to run, unless there's a custom dataStrategy + // since they may have different revalidation rules (i.e., single fetch) + if ( + !init.dataStrategy && + matchesToLoad.length === 0 && + revalidatingFetchers.length === 0 + ) { let updatedFetchers = markFetchRedirectsDone(); completeNavigation( location, @@ -3783,7 +3788,7 @@ export function createStaticHandler( ); // Short circuit if we have no loaders to run (query()) - if (matchesToLoad.length === 0) { + if (!dataStrategy && matchesToLoad.length === 0) { return { matches, // Add a null for all matched routes for proper revalidation on the client