From b6ed43d3bb6f28387efe2407fc3e211be512c98d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 24 Nov 2025 13:56:09 -0500 Subject: [PATCH 1/6] Remove old shouldRevalidate overwrite approach --- packages/react-router/lib/dom/ssr/routes.tsx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/react-router/lib/dom/ssr/routes.tsx b/packages/react-router/lib/dom/ssr/routes.tsx index 577cdf0624..0c1682c250 100644 --- a/packages/react-router/lib/dom/ssr/routes.tsx +++ b/packages/react-router/lib/dom/ssr/routes.tsx @@ -604,14 +604,6 @@ function getShouldRevalidateFunction( } } - // Single fetch revalidates by default, so override the RR default value which - // matches the multi-fetch behavior with `true` - if (ssr && route.shouldRevalidate) { - let fn = route.shouldRevalidate; - return (opts: ShouldRevalidateFunctionArgs) => - fn({ ...opts, defaultShouldRevalidate: true }); - } - return route.shouldRevalidate; } From 1868148d60687e53091aa1cb2acf1bf2a5417f0e Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 24 Nov 2025 14:17:06 -0500 Subject: [PATCH 2/6] Stabilise shouldCallHandler APIs --- .changeset/witty-ears-itch.md | 29 +++++++++++++++++++ .../router/should-revalidate-test.ts | 6 ++-- .../react-router/lib/dom/ssr/single-fetch.tsx | 20 ++++++------- packages/react-router/lib/router/router.ts | 22 +++++++------- packages/react-router/lib/router/utils.ts | 22 ++++++++++---- 5 files changed, 68 insertions(+), 31 deletions(-) create mode 100644 .changeset/witty-ears-itch.md diff --git a/.changeset/witty-ears-itch.md b/.changeset/witty-ears-itch.md new file mode 100644 index 0000000000..64a6582a76 --- /dev/null +++ b/.changeset/witty-ears-itch.md @@ -0,0 +1,29 @@ +--- +"react-router": minor +--- + +Stabilize the `dataStrategy` `match.shouldRevalidateArgs`/`match.shouldCallHandler()` APIs. + +- The `match.shouldLoad` API is now marked deprecated in favor of these more powerful alternatives +- If you're using this API in a custom `dataStrategy` today, you can swap to the new API at your convenience: + + ```tsx + // Before + const matchesToLoad = matches.filter((m) => m.shouldLoad); + + // After + const matchesToLoad = matches.filter((m) => m.shouldCallHandler()); + ``` + +- `match.shouldRevalidateArgs` is the argument that will be passed to the route `shouldRevaliate` function +- Combined with the parameter accepted by `match.shouldCallHandler`, you can define a custom revalidation behavior for your `dataStrategy`: + +```tsx +const matchesToLoad = matches.filter((m) => { + const defaultShouldRevalidate = customRevalidationBehavior( + match.shouldRevalidateArgs, + ); + return m.shouldCallHandler(defaultShouldRevalidate); + // The argument here will override the internal `defaultShouldRevalidate` value +}); +``` diff --git a/packages/react-router/__tests__/router/should-revalidate-test.ts b/packages/react-router/__tests__/router/should-revalidate-test.ts index 35a085c780..ad0ce40b31 100644 --- a/packages/react-router/__tests__/router/should-revalidate-test.ts +++ b/packages/react-router/__tests__/router/should-revalidate-test.ts @@ -1200,11 +1200,11 @@ describe("shouldRevalidate", () => { async dataStrategy({ request, matches }) { let keyedResults = {}; let matchesToLoad = matches.filter((match) => - match.unstable_shouldCallHandler( + match.shouldCallHandler( request.method === "POST" ? undefined - : !match.unstable_shouldRevalidateArgs?.actionStatus || - match.unstable_shouldRevalidateArgs.actionStatus < 400, + : !match.shouldRevalidateArgs?.actionStatus || + match.shouldRevalidateArgs.actionStatus < 400, ), ); await Promise.all( diff --git a/packages/react-router/lib/dom/ssr/single-fetch.tsx b/packages/react-router/lib/dom/ssr/single-fetch.tsx index bffc02708d..ced041546d 100644 --- a/packages/react-router/lib/dom/ssr/single-fetch.tsx +++ b/packages/react-router/lib/dom/ssr/single-fetch.tsx @@ -219,7 +219,7 @@ export function getSingleFetchDataStrategyImpl( let foundRevalidatingServerLoader = matches.some((m) => { let { hasLoader, hasClientLoader } = getRouteInfo(m); - return m.unstable_shouldCallHandler() && hasLoader && !hasClientLoader; + return m.shouldCallHandler() && hasLoader && !hasClientLoader; }); if (!ssr && !foundRevalidatingServerLoader) { // If this is SPA mode, there won't be any loaders below root and we'll @@ -282,7 +282,7 @@ async function singleFetchActionStrategy( fetchAndDecode: FetchAndDecodeFunction, basename: string | undefined, ) { - let actionMatch = args.matches.find((m) => m.unstable_shouldCallHandler()); + let actionMatch = args.matches.find((m) => m.shouldCallHandler()); invariant(actionMatch, "No action match found"); let actionStatus: number | undefined = undefined; let result = await actionMatch.resolve(async (handler) => { @@ -321,9 +321,7 @@ async function nonSsrStrategy( fetchAndDecode: FetchAndDecodeFunction, basename: string | undefined, ) { - let matchesToLoad = args.matches.filter((m) => - m.unstable_shouldCallHandler(), - ); + let matchesToLoad = args.matches.filter((m) => m.shouldCallHandler()); let results: Record = {}; await Promise.all( matchesToLoad.map((m) => @@ -385,15 +383,15 @@ async function singleFetchLoaderNavigationStrategy( getRouteInfo(m); let defaultShouldRevalidate = - !m.unstable_shouldRevalidateArgs || - m.unstable_shouldRevalidateArgs.actionStatus == null || - m.unstable_shouldRevalidateArgs.actionStatus < 400; - let shouldCall = m.unstable_shouldCallHandler(defaultShouldRevalidate); + !m.shouldRevalidateArgs || + m.shouldRevalidateArgs.actionStatus == null || + m.shouldRevalidateArgs.actionStatus < 400; + let shouldCall = m.shouldCallHandler(defaultShouldRevalidate); if (!shouldCall) { // If this route opted out, don't include in the .data request foundOptOutRoute ||= - m.unstable_shouldRevalidateArgs != null && // This is a revalidation, + m.shouldRevalidateArgs != null && // This is a revalidation, hasLoader && // for a route with a server loader, hasShouldRevalidate === true; // and a shouldRevalidate function return; @@ -538,7 +536,7 @@ async function singleFetchLoaderFetcherStrategy( fetchAndDecode: FetchAndDecodeFunction, basename: string | undefined, ) { - let fetcherMatch = args.matches.find((m) => m.unstable_shouldCallHandler()); + let fetcherMatch = args.matches.find((m) => m.shouldCallHandler()); invariant(fetcherMatch, "No fetcher match found"); let routeId = fetcherMatch.route.id; let result = await fetcherMatch.resolve(async (handler) => diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 70f2bf48d6..da9d2557fe 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -5695,7 +5695,7 @@ function runClientMiddlewarePipeline( ), // or the shallowest route that needs to load data Math.max( - matches.findIndex((m) => m.unstable_shouldCallHandler()), + matches.findIndex((m) => m.shouldCallHandler()), 0, ), ); @@ -5874,10 +5874,10 @@ function getDataStrategyMatch( lazyRoutePropertiesToSkip: string[], scopedContext: unknown, shouldLoad: boolean, - unstable_shouldRevalidateArgs: DataStrategyMatch["unstable_shouldRevalidateArgs"] = null, + shouldRevalidateArgs: DataStrategyMatch["shouldRevalidateArgs"] = null, ): DataStrategyMatch { // The hope here is to avoid a breaking change to the resolve behavior. - // Opt-ing into the `unstable_shouldCallHandler` API changes some nuanced behavior + // Opt-ing into the `shouldCallHandler` API changes some nuanced behavior // around when resolve calls through to the handler let isUsingNewApi = false; @@ -5893,20 +5893,20 @@ function getDataStrategyMatch( ...match, _lazyPromises, shouldLoad, - unstable_shouldRevalidateArgs, - unstable_shouldCallHandler(defaultShouldRevalidate) { + shouldRevalidateArgs, + shouldCallHandler(defaultShouldRevalidate) { isUsingNewApi = true; - if (!unstable_shouldRevalidateArgs) { + if (!shouldRevalidateArgs) { return shouldLoad; } if (typeof defaultShouldRevalidate === "boolean") { return shouldRevalidateLoader(match, { - ...unstable_shouldRevalidateArgs, + ...shouldRevalidateArgs, defaultShouldRevalidate, }); } - return shouldRevalidateLoader(match, unstable_shouldRevalidateArgs); + return shouldRevalidateLoader(match, shouldRevalidateArgs); }, resolve(handlerOverride) { let { lazy, loader, middleware } = match.route; @@ -5951,7 +5951,7 @@ function getTargetedDataStrategyMatches( targetMatch: AgnosticDataRouteMatch, lazyRoutePropertiesToSkip: string[], scopedContext: unknown, - shouldRevalidateArgs: DataStrategyMatch["unstable_shouldRevalidateArgs"] = null, + shouldRevalidateArgs: DataStrategyMatch["shouldRevalidateArgs"] = null, ): DataStrategyMatch[] { return matches.map((match) => { if (match.route.id !== targetMatch.route.id) { @@ -5960,8 +5960,8 @@ function getTargetedDataStrategyMatches( return { ...match, shouldLoad: false, - unstable_shouldRevalidateArgs: shouldRevalidateArgs, - unstable_shouldCallHandler: () => false, + shouldRevalidateArgs: shouldRevalidateArgs, + shouldCallHandler: () => false, _lazyPromises: getDataStrategyMatchLazyPromises( mapRouteProperties, manifest, diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts index b670d47781..0ec29fdc74 100644 --- a/packages/react-router/lib/router/utils.ts +++ b/packages/react-router/lib/router/utils.ts @@ -434,6 +434,8 @@ export interface DataStrategyMatch route: Promise | undefined; }; /** + * @deprecated Deprecated in favor of `shouldCallHandler` + * * A boolean value indicating whether this route handler should be called in * this pass. * @@ -459,12 +461,20 @@ export interface DataStrategyMatch * custom `shouldRevalidate` implementations) */ shouldLoad: boolean; - // This can be null for actions calls and for initial hydration calls - unstable_shouldRevalidateArgs: ShouldRevalidateFunctionArgs | null; - // This function will use a scoped version of `shouldRevalidateArgs` because - // they are read-only but let the user provide an optional override value for - // `defaultShouldRevalidate` if they choose - unstable_shouldCallHandler(defaultShouldRevalidate?: boolean): boolean; + /** + * Arguments passed to the `shouldRevalidate` function for this `loader` execution. + * Will be `null` if this is not a revalidating loader {@link DataStrategyMatch}. + */ + shouldRevalidateArgs: ShouldRevalidateFunctionArgs | null; + /** + * Determine if this route's handler should be called during this `dataStrategy` + * execution. Calling it with no arguments will leverage the default revalidation + * behavior. You can pass your own `defaultShouldRevalidate` value if you wish + * to change the default revalidation behavior with your `dataStrategy`. + * + * @param defaultShouldRevalidate `defaultShouldRevalidate` override value (optional) + */ + shouldCallHandler(defaultShouldRevalidate?: boolean): boolean; /** * An async function that will resolve any `route.lazy` implementations and * execute the route's handler (if necessary), returning a {@link DataStrategyResult} From 388ec2453066c2263dd5be3a05862de95550ef71 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 24 Nov 2025 14:44:50 -0500 Subject: [PATCH 3/6] Add E2E tests on action 4xx defaultShouldRevalidate --- integration/single-fetch-test.ts | 101 +++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index 222f060a99..b02c25643c 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -736,6 +736,107 @@ test.describe("single-fetch", () => { expect(urls).toEqual([]); }); + test("provides proper defaultShouldRevalidate value on 4xx/5xx action responses", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/action.tsx": js` + import { Form, Link, useActionData, useLoaderData, useNavigation, data } from 'react-router'; + + export async function action({ request }) { + let fd = await request.formData(); + if (fd.get('throw') === "5xx") { + throw data("Thrown 500", { status: 500 }); + } + if (fd.get('throw') === "4xx") { + throw data("Thrown 400", { status: 400 }); + } + if (fd.get('return') === "5xx") { + return data("Returned 500", { status: 500 }); + } + if (fd.get('return') === "4xx") { + return data("Returned 400", { status: 400 }); + } + return null; + } + + let count = 0; + export function loader() { + return { count: ++count }; + } + + export function shouldRevalidate({ defaultShouldRevalidate }) { + return defaultShouldRevalidate; + } + + export default function Comp() { + let navigation = useNavigation(); + let data = useLoaderData(); + return ( +
+ + + + +

{data.count}

+ {navigation.state === "idle" ?

idle

: null} +
+ ); + } + + export function ErrorBoundary() { + return ( +
+

Error

+ Back +
+ ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + console.error = () => {}; + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/action"); + expect(await app.getHtml("#data")).toContain("1"); + expect(urls).toEqual([]); + + await page.click('button[name="return"][value="5xx"]'); + await page.waitForSelector("#idle"); + expect(await app.getHtml("#data")).toContain("1"); + expect(urls).toEqual([]); + + await page.click('button[name="return"][value="4xx"]'); + await page.waitForSelector("#idle"); + expect(await app.getHtml("#data")).toContain("1"); + expect(urls).toEqual([]); + + await page.click('button[name="throw"][value="5xx"]'); + await page.waitForSelector("#error"); + expect(urls).toEqual([]); + + await app.clickLink("/action"); + await page.waitForSelector("#data"); + expect(await app.getHtml("#data")).toContain("2"); + urls = []; + + await page.click('button[name="throw"][value="4xx"]'); + await page.waitForSelector("#error"); + expect(urls).toEqual([]); + }); + test("returns headers correctly for singular loader and action calls", async () => { let fixture = await createFixture({ files: { From acb321b866d5b4b6d4b98288956ca4381261d251 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 24 Nov 2025 14:49:01 -0500 Subject: [PATCH 4/6] Add changeset --- .changeset/bright-brooms-hammer.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/bright-brooms-hammer.md diff --git a/.changeset/bright-brooms-hammer.md b/.changeset/bright-brooms-hammer.md new file mode 100644 index 0000000000..527622f92d --- /dev/null +++ b/.changeset/bright-brooms-hammer.md @@ -0,0 +1,7 @@ +--- +"react-router": patch +--- + +Fix a Framework Mode bug where the `defaultShouldRevalidate` parameter to `shouldRevalidate` would not be correct after `action` returned a 4xx/5xx response + +- If your `shouldRevalidate` function relied on that parameter, you may have seen unintended revalidations From 8dfe36dba80454248846b9717df82f88de78b9ea Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 24 Nov 2025 14:50:29 -0500 Subject: [PATCH 5/6] Update --- .changeset/bright-brooms-hammer.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/bright-brooms-hammer.md b/.changeset/bright-brooms-hammer.md index 527622f92d..1c479745be 100644 --- a/.changeset/bright-brooms-hammer.md +++ b/.changeset/bright-brooms-hammer.md @@ -2,6 +2,6 @@ "react-router": patch --- -Fix a Framework Mode bug where the `defaultShouldRevalidate` parameter to `shouldRevalidate` would not be correct after `action` returned a 4xx/5xx response +Fix a Framework Mode bug where the `defaultShouldRevalidate` parameter to `shouldRevalidate` would not be correct after `action` returned a 4xx/5xx response (`true` when it should have been `false`) - If your `shouldRevalidate` function relied on that parameter, you may have seen unintended revalidations From 83bd9e6e2a187bc614aa7f6884a6c605adf387d9 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 24 Nov 2025 15:29:58 -0500 Subject: [PATCH 6/6] Additional tests for shouldCallHandler --- .../__tests__/router/data-strategy-test.ts | 411 ++++++++++++++++++ .../router/utils/data-router-setup.ts | 2 +- 2 files changed, 412 insertions(+), 1 deletion(-) diff --git a/packages/react-router/__tests__/router/data-strategy-test.ts b/packages/react-router/__tests__/router/data-strategy-test.ts index 3821fec973..033d66b4c0 100644 --- a/packages/react-router/__tests__/router/data-strategy-test.ts +++ b/packages/react-router/__tests__/router/data-strategy-test.ts @@ -32,6 +32,27 @@ describe("router dataStrategy", () => { ); } + function keyedResultsUsingShouldCallHandler( + matchesToLoad: DataStrategyMatch[], + results: DataStrategyResult[], + ) { + if (matchesToLoad.length !== results.length) { + throw new Error( + `Mismatched results length: expected ${matchesToLoad.length} but got ${results.length}`, + ); + } + return results.reduce( + (acc, r, i) => + Object.assign( + acc, + matchesToLoad[i].shouldCallHandler() + ? { [matchesToLoad[i].route.id]: r } + : {}, + ), + {}, + ); + } + describe("loaders", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => @@ -578,6 +599,112 @@ describe("router dataStrategy", () => { }); }); + it("indicates which routes need to load via match.shouldCallHandler()", async () => { + let shouldCallHandlerValues: Record[] = []; + let dataStrategy = jest.fn< + ReturnType, + Parameters + >(async ({ request, matches }) => { + let values = matches.reduce( + (acc, m) => + Object.assign(acc, { + [m.route.id]: m.shouldCallHandler(), + }), + {}, + ); + shouldCallHandlerValues.push(values); + let matchesToLoad = matches.filter((m) => m.shouldCallHandler()); + let results = await Promise.all(matchesToLoad.map((m) => m.resolve())); + return keyedResultsUsingShouldCallHandler(matchesToLoad, results); + }); + let [lazy, lazyDeferred] = createAsyncStub(); + let t = setup({ + routes: [ + { + id: "root", + path: "/", + loader: true, + children: [ + { + id: "parent", + path: "parent", + loader: true, + action: true, + children: [ + { + id: "child", + path: "child", + lazy, + }, + ], + }, + ], + }, + ], + dataStrategy, + hydrationData: { + // don't call dataStrategy on hydration + loaderData: { root: null }, + }, + }); + + let A = await t.navigate("/"); + expect(shouldCallHandlerValues[0]).toEqual({ root: true }); + await A.loaders.root.resolve("ROOT"); + expect(t.router.state.loaderData).toMatchObject({ + root: "ROOT", + }); + + let B = await t.navigate("/parent"); + expect(shouldCallHandlerValues[1]).toEqual({ root: false, parent: true }); + await B.loaders.parent.resolve("PARENT"); + expect(t.router.state.loaderData).toMatchObject({ + root: "ROOT", + parent: "PARENT", + }); + + await t.navigate("/parent/child"); + expect(shouldCallHandlerValues[2]).toEqual({ + root: false, + parent: false, + child: true, + }); + await lazyDeferred.resolve({ + action: () => "CHILD ACTION", + loader: () => "CHILD", + shouldRevalidate: () => false, + }); + expect(t.router.state.loaderData).toMatchObject({ + root: "ROOT", + parent: "PARENT", + child: "CHILD", + }); + + await t.navigate("/parent/child", { + formMethod: "post", + formData: createFormData({}), + }); + await tick(); + expect(shouldCallHandlerValues[3]).toEqual({ + root: false, + parent: false, + child: true, + }); + expect(shouldCallHandlerValues[4]).toEqual({ + root: true, + parent: true, + child: false, + }); + expect(t.router.state.actionData).toMatchObject({ + child: "CHILD ACTION", + }); + expect(t.router.state.loaderData).toMatchObject({ + root: "ROOT", + parent: "PARENT", + child: "CHILD", + }); + }); + it("does not short circuit when there are no matchesToLoad", async () => { let dataStrategy = mockDataStrategy(async ({ matches }) => { let results = await Promise.all( @@ -1295,6 +1422,109 @@ describe("router dataStrategy", () => { }); }); + it("allows middleware/context implementations when some routes don't need to revalidate (using shouldCallHandler)", async () => { + let t = setup({ + routes: [ + { + path: "/", + }, + { + id: "parent", + path: "/parent", + loader: true, + handle: { + context: { + parent: () => ({ id: "parent" }), + }, + middleware(context) { + context.parent.whatever = "PARENT MIDDLEWARE"; + }, + }, + children: [ + { + id: "child", + path: "child", + loader: true, + handle: { + context: { + child: () => ({ id: "child" }), + }, + middleware(context) { + context.child.whatever = "CHILD MIDDLEWARE"; + }, + }, + }, + ], + }, + ], + async dataStrategy({ matches }) { + // Run context/middleware sequentially + let context = matches.reduce((acc, m) => { + if (m.route.handle?.context) { + let matchContext = Object.entries(m.route.handle.context).reduce( + (acc, [key, value]) => + Object.assign(acc, { + // @ts-expect-error + [key]: value(), + }), + {}, + ); + Object.assign(acc, matchContext); + } + if (m.route.handle?.middleware) { + m.route.handle.middleware(acc); + } + return acc; + }, {}); + + // Run loaders in parallel only exposing contexts from above + let matchesToLoad = matches.filter((m) => m.shouldCallHandler()); + let results = await Promise.all( + matchesToLoad.map((m) => + m.resolve((callHandler) => callHandler(context)), + ), + ); + return keyedResultsUsingShouldCallHandler(matchesToLoad, results); + }, + }); + + let A = await t.navigate("/parent"); + await A.loaders.parent.resolve("PARENT"); + expect(t.router.state.navigation.state).toBe("idle"); + expect(t.router.state.loaderData).toMatchObject({ + parent: "PARENT", + }); + + let B = await t.navigate("/parent/child"); + + // Loaders are called with context from their level and above, and + // context reflects any values set by middleware + expect(B.loaders.child.stub).toHaveBeenCalledWith( + expect.objectContaining({ + request: expect.any(Request), + params: expect.any(Object), + }), + { + parent: { + id: "parent", + whatever: "PARENT MIDDLEWARE", + }, + child: { + id: "child", + whatever: "CHILD MIDDLEWARE", + }, + }, + ); + + await B.loaders.child.resolve("CHILD"); + expect(t.router.state.navigation.state).toBe("idle"); + + expect(t.router.state.loaderData).toMatchObject({ + parent: "PARENT", + child: "CHILD", + }); + }); + it("allows automatic caching of loader results", async () => { let cache: Record = {}; let t = setup({ @@ -1407,5 +1637,186 @@ describe("router dataStrategy", () => { }, }); }); + + it("allows automatic caching of loader results (using shouldCallHandler)", async () => { + let cache: Record = {}; + let t = setup({ + routes: [ + { + path: "/", + }, + { + id: "parent", + path: "/parent", + loader: true, + handle: { + cacheKey: (url: string) => new URL(url).pathname, + }, + children: [ + { + id: "child", + path: "child", + loader: true, + action: true, + }, + ], + }, + ], + async dataStrategy({ request, matches }) { + const getCacheKey = (m: DataStrategyMatch) => + m.route.handle?.cacheKey + ? [m.route.id, m.route.handle.cacheKey(request.url)].join("-") + : null; + + if (request.method !== "GET") { + // invalidate on actions + cache = {}; + } + + let matchesToLoad = matches.filter((m) => m.shouldCallHandler()); + let results = await Promise.all( + matchesToLoad.map(async (m) => { + return m.resolve(async (handler) => { + let key = getCacheKey(m); + if (key && cache[key]) { + return cache[key]; + } + + let dsResult = await handler(); + if (key && request.method === "GET") { + cache[key] = dsResult; + } + + return dsResult; + }); + }), + ); + return keyedResultsUsingShouldCallHandler(matchesToLoad, results); + }, + }); + + let A = await t.navigate("/parent/child"); + await A.loaders.parent.resolve("PARENT"); + await A.loaders.child.resolve("CHILD"); + + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + loaderData: { + parent: "PARENT", + child: "CHILD", + }, + }); + + // Changing search params should force revalidation, but pathname-based + // cache will serve the old data + let B = await t.navigate("/parent/child?a=b"); + await B.loaders.child.resolve("CHILD*"); + + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + loaderData: { + parent: "PARENT", + child: "CHILD*", + }, + }); + + // Useless resolution - handler was never called for parent + await B.loaders.parent.resolve("PARENT*"); + + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + loaderData: { + parent: "PARENT", + child: "CHILD*", + }, + }); + + // Action to invalidate the cache + let C = await t.navigate("/parent/child?a=b", { + formMethod: "post", + formData: createFormData({}), + }); + await C.actions.child.resolve("ACTION"); + await C.loaders.parent.resolve("PARENT**"); + await C.loaders.child.resolve("CHILD**"); + + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + actionData: { + child: "ACTION", + }, + loaderData: { + parent: "PARENT**", + child: "CHILD**", + }, + }); + }); + + it("allows defining a custom revalidation behavior", async () => { + let t = setup({ + routes: [ + { + id: "index", + path: "/", + action: true, + loader: true, + }, + ], + async dataStrategy({ request, matches, context }) { + if (request.method !== "GET") { + context.actionMethod = request.method; + } + // Don't revalidate on PUT requests + let defaultShouldRevalidate = + context.actionMethod === "PUT" ? false : undefined; + let matchesToLoad = matches.filter((m) => + m.shouldCallHandler(defaultShouldRevalidate), + ); + let results = await Promise.all( + matchesToLoad.map((m) => m.resolve()), + ); + return keyedResultsUsingShouldCallHandler(matchesToLoad, results); + }, + hydrationData: { + loaderData: { + index: "INDEX", + }, + }, + }); + + let A = await t.navigate("/", { + formMethod: "post", + formData: createFormData({}), + }); + await A.actions.index.resolve("ACTION1"); + await A.loaders.index.resolve("INDEX1"); + expect(t.router.state.navigation.state).toBe("idle"); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + actionData: { + index: "ACTION1", + }, + loaderData: { + index: "INDEX1", + }, + }); + + let B = await t.navigate("/", { + formMethod: "put", + formData: createFormData({}), + }); + await B.actions.index.resolve("ACTION2"); + //await B.loaders.index.resolve("INDEX2"); // no-op + expect(t.router.state.navigation.state).toBe("idle"); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + actionData: { + index: "ACTION2", + }, + loaderData: { + index: "INDEX1", + }, + }); + }); }); }); diff --git a/packages/react-router/__tests__/router/utils/data-router-setup.ts b/packages/react-router/__tests__/router/utils/data-router-setup.ts index dffbd34c95..df2c2c2145 100644 --- a/packages/react-router/__tests__/router/utils/data-router-setup.ts +++ b/packages/react-router/__tests__/router/utils/data-router-setup.ts @@ -717,7 +717,7 @@ export function setup({ history, router: currentRouter, get fetchers() { - let fetchers = {}; + let fetchers: Record = {}; currentRouter?.state.fetchers.forEach((f, key) => { fetchers[key] = { ...f,