diff --git a/.changeset/fair-weeks-beam.md b/.changeset/fair-weeks-beam.md new file mode 100644 index 0000000000..b2595f7ec6 --- /dev/null +++ b/.changeset/fair-weeks-beam.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Do not short circuit navigation logic if no matches are found requiring revalidation if a user-provided `dataStrategy` exists since it may want to change the revalidation behavior diff --git a/packages/router/__tests__/data-strategy-test.ts b/packages/router/__tests__/data-strategy-test.ts index 6039f9640e..f8d854a25e 100644 --- a/packages/router/__tests__/data-strategy-test.ts +++ b/packages/router/__tests__/data-strategy-test.ts @@ -569,6 +569,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/router/router.ts b/packages/router/router.ts index 933c283dca..c440b87323 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1943,8 +1943,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, @@ -3944,7 +3949,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