diff --git a/.changeset/four-books-bow.md b/.changeset/four-books-bow.md new file mode 100644 index 0000000000..370823e8dd --- /dev/null +++ b/.changeset/four-books-bow.md @@ -0,0 +1,14 @@ +--- +"@remix-run/router": patch +--- + +Update the `unstable_dataStrategy` API to allow for more advanced implementations + +- Rename `unstable_HandlerResult` to `unstable_DataStrategyResult` +- The return signature has changed from a parallel array of `unstable_DataStrategyResult[]` (parallel to `matches`) to a key/value object of `routeId => unstable_DataStrategyResult` + - This allows you to more easily decide to opt-into or out-of revalidating data that may not have been revalidated by default (via `match.shouldLoad`) + - ⚠️ This is a breaking change if you've currently adopted `unstable_dataStrategy` +- Added a new `fetcherKey` parameter to `unstable_dataStrategy` to allow differentiation from navigational and fetcher calls +- You should now return/throw a result from your `handlerOverride` instead of returning a `DataStrategyResult` + - If you are aggregating the results of `match.resolve()` into a final results object you should not need to think about the `DataStrategyResult` type + - If you are manually filling your results object from within your `handlerOverride`, then you will need to assign a `DataStrategyResult` as the value so React Router knows if it's a successful execution or an error. diff --git a/docs/routers/create-browser-router.md b/docs/routers/create-browser-router.md index 87bd13fca3..a2f2cb64d9 100644 --- a/docs/routers/create-browser-router.md +++ b/docs/routers/create-browser-router.md @@ -199,7 +199,7 @@ The `unstable_dataStrategy` option gives you full control over how your loaders ```ts interface DataStrategyFunction { (args: DataStrategyFunctionArgs): Promise< - HandlerResult[] + Record >; } @@ -208,6 +208,7 @@ interface DataStrategyFunctionArgs { params: Params; context?: Context; matches: DataStrategyMatch[]; + fetcherKey: string | null; } interface DataStrategyMatch @@ -219,34 +220,36 @@ interface DataStrategyMatch resolve: ( handlerOverride?: ( handler: (ctx?: unknown) => DataFunctionReturnValue - ) => Promise - ) => Promise; + ) => Promise + ) => Promise; } -interface HandlerResult { +interface DataStrategyResult { type: "data" | "error"; - result: any; // data, Error, Response, DeferredData - status?: number; + result: unknown; // data, Error, Response, DeferredData, DataWithResponseInit } ``` ### Overview -`unstable_dataStrategy` receives the same arguments as a `loader`/`action` (`request`, `params`) but it also receives a `matches` array which is an array of the matched routes where each match is extended with 2 new fields for use in the data strategy function: - -- **`match.resolve`** - An async function that will resolve any `route.lazy` implementations and execute the route's handler (if necessary), returning a `HandlerResult` - - You should call `match.resolve` for _all_ matches every time to ensure that all lazy routes are properly resolved - - This does not mean you're calling the loader/action (the "handler") - `resolve` will only call the `handler` internally if needed and if you don't pass your own `handlerOverride` function parameter - - See the examples below for how to implement custom handler execution via `match.resolve` -- **`match.shouldLoad`** - A boolean value indicating whether this route handler needs to be called in this pass - - The `matches` array always includes _all_ matched routes even when only _some_ route handlers need to be called so that things like middleware can be implemented - - `shouldLoad` is usually only interesting if you are skipping the route handler entirely and implementing custom handler logic - since it lets you determine if that custom logic should run for this route or not - - For example: - - If you are on `/parent/child/a` and you navigate to `/parent/child/b` - you'll get an array of three matches (`[parent, child, b]`), but only `b` will have `shouldLoad=true` because the data for `parent` and `child` is already loaded - - If you are on `/parent/child/a` and you submit to `a`'s `action`, then only `a` will have `shouldLoad=true` for the action execution of `dataStrategy` - - After the `action`, `dataStrategy` will be called again for the `loader` revalidation, and all matches will have `shouldLoad=true` (assuming no custom `shouldRevalidate` implementations) - -The `dataStrategy` function should return a parallel array of `HandlerResult` instances, which indicates if the handler was successful or not. If the returned `handlerResult.result` is a `Response`, React Router will unwrap it for you (via `res.json` or `res.text`). If you need to do custom decoding of a `Response` but preserve the status code, you can return the decoded value in `handlerResult.result` and send the status along via `handlerResult.status` (for example, when using the `future.v7_skipActionRevalidation` flag). `match.resolve()` will return a `HandlerResult` if you are not passing it a handler override function. If you are, then you need to wrap the `handler` result in a `HandlerResult` (see examples below). +`unstable_dataStrategy` receives the same arguments as a `loader`/`action` (`request`, `params`) but it also receives 2 new parameters: `matches` and `fetcherKey`: + +- **`matches`** - An array of the matched routes where each match is extended with 2 new fields for use in the data strategy function: + - **`match.shouldLoad`** - A boolean value indicating whether this route handler should be called in this pass + - The `matches` array always includes _all_ matched routes even when only _some_ route handlers need to be called so that things like middleware can be implemented + - `shouldLoad` is usually only interesting if you are skipping the route handler entirely and implementing custom handler logic - since it lets you determine if that custom logic should run for this route or not + - For example: + - If you are on `/parent/child/a` and you navigate to `/parent/child/b` - you'll get an array of three matches (`[parent, child, b]`), but only `b` will have `shouldLoad=true` because the data for `parent` and `child` is already loaded + - If you are on `/parent/child/a` and you submit to `a`'s `action`, then only `a` will have `shouldLoad=true` for the action execution of `dataStrategy` + - After the `action`, `dataStrategy` will be called again for the `loader` revalidation, and all matches will have `shouldLoad=true` (assuming no custom `shouldRevalidate` implementations) + - **`match.resolve`** - An async function that will resolve any `route.lazy` implementations and execute the route's handler (if necessary), returning a `DataStrategyResult` + - Calling `match.resolve` does not mean you're calling the `loader`/`action` (the "handler") - `resolve` will only call the `handler` internally if needed _and_ if you don't pass your own `handlerOverride` function parameter + - It is safe to call `match.resolve` for all matches, even if they have `shouldLoad=false`, and it will no-op if no loading is required + - You should generally always call `match.resolve()` for `shouldLoad:true` routes to ensure that any `route.lazy` implementations are processed + - See the examples below for how to implement custom handler execution via `match.resolve` +- **`fetcherKey`** - The key of the fetcher we are calling `unstable_dataStrategy` for, otherwise `null` for navigational executions + +The `dataStrategy` function should return a key/value object of `routeId -> DataStrategyResult` and should include entries for any routes where a handler was executed. A `DataStrategyResult` indicates if the handler was successful or not based on the `DataStrategyResult["type"]` field. If the returned `DataStrategyResult["result"]` is a `Response`, React Router will unwrap it for you (via `res.json` or `res.text`). If you need to do custom decoding of a `Response` but want to preserve the status code, you can use the `unstable_data` utility to return your decoded data along with a `ResponseInit`. ### Example Use Cases @@ -256,18 +259,61 @@ In the simplest case, let's look at hooking into this API to add some logging fo ```ts let router = createBrowserRouter(routes, { - unstable_dataStrategy({ request, matches }) { - return Promise.all( - matches.map(async (match) => { - console.log(`Processing route ${match.route.id}`); + async unstable_dataStrategy({ request, matches }) { + // Grab only the matches we need to run handlers for + const matchesToLoad = matches.filter( + (m) => m.shouldLoad + ); + // Run the handlers in parallel, logging before and after + const results = await Promise.all( + matchesToLoad.map(async (match) => { + console.log(`Processing ${match.route.id}`); // Don't override anything - just resolve route.lazy + call loader - let result = await match.resolve(); - console.log( - `Done processing route ${match.route.id}` - ); + const result = await match.resolve(); return result; }) ); + + // Aggregate the results into a bn object of `routeId -> DataStrategyResult` + return results.reduce( + (acc, result, i) => + Object.assign(acc, { + [matchesToLoad[i].route.id]: result, + }), + {} + ); + }, +}); +``` + +If you want to avoid the `reduce`, you can manually build up the `results` object, but you'll need to construct the `DataStrategyResult` manually - indicating if the handler was successful or not: + +```ts +let router = createBrowserRouter(routes, { + async unstable_dataStrategy({ request, matches }) { + const matchesToLoad = matches.filter( + (m) => m.shouldLoad + ); + const results = {}; + await Promise.all( + matchesToLoad.map(async (match) => { + console.log(`Processing ${match.route.id}`); + try { + const result = await match.resolve(); + results[match.route.id] = { + type: "data", + result, + }; + } catch (e) { + results[match.route.id] = { + type: "error", + result: e, + }; + } + }) + ); + + return results; }, }); ``` @@ -324,16 +370,23 @@ let router = createBrowserRouter(routes, { } // Run loaders in parallel with the `context` value - return Promise.all( - matches.map((match, i) => - match.resolve(async (handler) => { + let matchesToLoad = matches.filter((m) => m.shouldLoad); + let results = await Promise.all( + matchesToLoad.map((match, i) => + match.resolve((handler) => { // Whatever you pass to `handler` will be passed as the 2nd parameter // to your loader/action - let result = await handler(context); - return { type: "data", result }; + return handler(context); }) ) ); + return results.reduce( + (acc, result, i) => + Object.assign(acc, { + [matchesToLoad[i].route.id]: result, + }), + {} + ); }, }); ``` @@ -377,7 +430,8 @@ let router = createBrowserRouter(routes, { // Compose route fragments into a single GQL payload let gql = getFragmentsFromRouteHandles(matches); let data = await fetchGql(gql); - // Parse results back out into individual route level HandlerResult's + // Parse results back out into individual route level `DataStrategyResult`'s + // keyed by `routeId` let results = parseResultsFromGql(data); return results; }, diff --git a/package.json b/package.json index 4bf46e006f..e1c0dd4700 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "57.2 kB" + "none": "58.1 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "15.0 kB" diff --git a/packages/react-router-dom-v5-compat/index.ts b/packages/react-router-dom-v5-compat/index.ts index 1486b8a9f2..5c1dc7e565 100644 --- a/packages/react-router-dom-v5-compat/index.ts +++ b/packages/react-router-dom-v5-compat/index.ts @@ -54,6 +54,7 @@ export type { unstable_DataStrategyFunction, unstable_DataStrategyFunctionArgs, unstable_DataStrategyMatch, + unstable_DataStrategyResult, DataRouteMatch, DataRouteObject, ErrorResponse, @@ -112,7 +113,6 @@ export type { UIMatch, Blocker, BlockerFunction, - unstable_HandlerResult, } from "./react-router-dom"; export { AbortedDeferredError, diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 6fba1e5265..5edf5889b2 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -16,6 +16,7 @@ import type { RouterProps, RouterProviderProps, To, + unstable_DataStrategyFunction, unstable_PatchRoutesOnNavigationFunction, } from "react-router"; import { @@ -38,9 +39,6 @@ import { } from "react-router"; import type { BrowserHistory, - unstable_DataStrategyFunction, - unstable_DataStrategyFunctionArgs, - unstable_DataStrategyMatch, Fetcher, FormEncType, FormMethod, @@ -89,9 +87,6 @@ import { //////////////////////////////////////////////////////////////////////////////// export type { - unstable_DataStrategyFunction, - unstable_DataStrategyFunctionArgs, - unstable_DataStrategyMatch, FormEncType, FormMethod, GetScrollRestorationKeyFunction, @@ -111,6 +106,10 @@ export type { BlockerFunction, DataRouteMatch, DataRouteObject, + unstable_DataStrategyFunction, + unstable_DataStrategyFunctionArgs, + unstable_DataStrategyMatch, + unstable_DataStrategyResult, ErrorResponse, Fetcher, FutureConfig, @@ -152,7 +151,6 @@ export type { ShouldRevalidateFunctionArgs, To, UIMatch, - unstable_HandlerResult, unstable_PatchRoutesOnNavigationFunction, } from "react-router"; export { diff --git a/packages/react-router-native/index.tsx b/packages/react-router-native/index.tsx index d950927223..6a9445c085 100644 --- a/packages/react-router-native/index.tsx +++ b/packages/react-router-native/index.tsx @@ -30,6 +30,7 @@ export type { unstable_DataStrategyFunction, unstable_DataStrategyFunctionArgs, unstable_DataStrategyMatch, + unstable_DataStrategyResult, ErrorResponse, Fetcher, FutureConfig, @@ -71,7 +72,6 @@ export type { ShouldRevalidateFunctionArgs, To, UIMatch, - unstable_HandlerResult, } from "react-router"; export { AbortedDeferredError, diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index 6ddef0c1b0..56a7cadc19 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -7,6 +7,7 @@ import type { unstable_DataStrategyFunction, unstable_DataStrategyFunctionArgs, unstable_DataStrategyMatch, + unstable_DataStrategyResult, ErrorResponse, Fetcher, HydrationState, @@ -31,7 +32,6 @@ import type { ShouldRevalidateFunctionArgs, To, UIMatch, - unstable_HandlerResult, unstable_AgnosticPatchRoutesOnNavigationFunction, } from "@remix-run/router"; import { @@ -139,6 +139,7 @@ export type { unstable_DataStrategyFunction, unstable_DataStrategyFunctionArgs, unstable_DataStrategyMatch, + unstable_DataStrategyResult, ErrorResponse, Fetcher, FutureConfig, @@ -182,7 +183,6 @@ export type { UIMatch, Blocker, BlockerFunction, - unstable_HandlerResult, }; export { AbortedDeferredError, diff --git a/packages/router/__tests__/data-strategy-test.ts b/packages/router/__tests__/data-strategy-test.ts index 6cdff873bc..6039f9640e 100644 --- a/packages/router/__tests__/data-strategy-test.ts +++ b/packages/router/__tests__/data-strategy-test.ts @@ -1,4 +1,8 @@ -import type { DataStrategyFunction, DataStrategyMatch } from "../utils"; +import type { + DataStrategyFunction, + DataStrategyMatch, + DataStrategyResult, +} from "../utils"; import { json } from "../utils"; import { createDeferred, setup } from "./utils/data-router-setup"; import { createFormData, tick } from "./utils/utils"; @@ -11,10 +15,26 @@ describe("router dataStrategy", () => { >(fn); } + function keyedResults( + matches: DataStrategyMatch[], + results: DataStrategyResult[] + ) { + return results.reduce( + (acc, r, i) => + Object.assign( + acc, + matches[i].shouldLoad ? { [matches[i].route.id]: r } : {} + ), + {} + ); + } + describe("loaders", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -72,7 +92,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -142,13 +164,10 @@ describe("router dataStrategy", () => { matches.map((m) => m.resolve(async (handler) => { let result = await handler(); - return { - type: "data", - result: `Route ID "${m.route.id}" returned "${result}"`, - }; + return `Route ID "${m.route.id}" returned "${result}"`; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -160,6 +179,38 @@ describe("router dataStrategy", () => { }); }); + it("should allow custom implementations to override default behavior when erroring", async () => { + let t = setup({ + routes: [ + { + path: "/", + }, + { + id: "test", + path: "/test", + loader: true, + hasErrorBoundary: true, + }, + ], + async dataStrategy({ matches }) { + return Promise.all( + matches.map((m) => + m.resolve(async () => { + throw new Error(`Route ID "${m.route.id}" errored!`); + }) + ) + ).then((results) => keyedResults(matches, results)); + }, + }); + + let A = await t.navigate("/test"); + await A.loaders.test.resolve("TEST"); + + expect(t.router.state.errors).toMatchObject({ + test: new Error('Route ID "test" errored!'), + }); + }); + it("should allow custom implementations to override default behavior with lazy", async () => { let t = setup({ routes: [ @@ -177,13 +228,10 @@ describe("router dataStrategy", () => { matches.map((m) => m.resolve(async (handler) => { let result = await handler(); - return { - type: "data", - result: `Route ID "${m.route.id}" returned "${result}"`, - }; + return `Route ID "${m.route.id}" returned "${result}"`; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -220,7 +268,9 @@ describe("router dataStrategy", () => { }, ], dataStrategy({ matches }) { - return Promise.all(matches.map(async (match) => match.resolve())); + return Promise.all( + matches.map(async (match) => match.resolve()) + ).then((results) => keyedResults(matches, results)); }, }); @@ -265,7 +315,7 @@ describe("router dataStrategy", () => { }; }); }) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -315,7 +365,7 @@ describe("router dataStrategy", () => { }); }); - it("throws an error if an implementation does not call resolve", async () => { + it("does not require resolve to be called if a match is not being loaded", async () => { let t = setup({ routes: [ { @@ -335,35 +385,47 @@ describe("router dataStrategy", () => { ], }, ], - // @ts-expect-error - dataStrategy({ matches }) { + dataStrategy({ matches, request }) { return Promise.all( matches.map(async (match) => { - if (match.route.id === "child") { - // noop to cause error - return "forgot to load child"; + if ( + request.url.endsWith("/parent/child") && + match.route.id === "parent" + ) { + return undefined; } return match.resolve(); }) + ).then((results) => + // @ts-expect-error + keyedResults(matches, results) ); }, }); - let A = await t.navigate("/parent/child"); + let A = await t.navigate("/parent"); await A.loaders.parent.resolve("PARENT"); - expect(t.router.state).toMatchObject({ - actionData: null, - errors: { - parent: new Error( - '`match.resolve()` was not called for route id "child". ' + - "You must call `match.resolve()` on every match passed " + - "to `dataStrategy` to ensure all routes are properly loaded." - ), + errors: null, + loaderData: { + parent: "PARENT", + }, + navigation: { + state: "idle", }, + }); + + let B = await t.navigate("/parent/child"); + await B.lazy.child.resolve({ loader: () => "CHILD" }); + + // no-op + await B.loaders.parent.resolve("XXX"); + + expect(t.router.state).toMatchObject({ + errors: null, loaderData: { - child: undefined, - parent: undefined, + child: "CHILD", + parent: "PARENT", }, navigation: { state: "idle", @@ -376,7 +438,9 @@ describe("router dataStrategy", () => { ReturnType, Parameters >(({ matches }) => { - return Promise.all(matches.map((m) => m.resolve())); + return Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ); }); let t = setup({ routes: [ @@ -510,7 +574,9 @@ describe("router dataStrategy", () => { describe("actions", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -552,7 +618,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -597,7 +665,9 @@ describe("router dataStrategy", () => { describe("loaders", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -638,7 +708,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -680,7 +752,9 @@ describe("router dataStrategy", () => { describe("actions", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -724,7 +798,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -800,18 +876,15 @@ describe("router dataStrategy", () => { ) { let str = await result.text(); return { - type: "data", - result: { - original: str, - reversed: str.split("").reverse().join(""), - }, + original: str, + reversed: str.split("").reverse().join(""), }; } // This will be a JSON response we expect to be decoded the normal way - return { type: "data", result }; + return result; }); }) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -832,6 +905,7 @@ describe("router dataStrategy", () => { }); }); + jest.setTimeout(10000000); it("allows a single-fetch type approach", async () => { let t = setup({ routes: [ @@ -866,7 +940,7 @@ describe("router dataStrategy", () => { // the single fetch response and return it's promise let dfd = createDeferred(); routeDeferreds.set(m.route.id, dfd); - return dfd.promise; + return dfd.promise as Promise; }) ); @@ -876,14 +950,15 @@ describe("router dataStrategy", () => { parent: "PARENT", child: "CHILD", }, - errors: null, }; // Resolve the deferred's above and return the mapped match promises routeDeferreds.forEach((dfd, routeId) => - dfd.resolve({ type: "data", result: result.loaderData[routeId] }) + dfd.resolve(result.loaderData[routeId]) + ); + return Promise.all(matchPromises).then((results) => + keyedResults(matches, results) ); - return Promise.all(matchPromises); }, }); @@ -965,10 +1040,11 @@ describe("router dataStrategy", () => { }); return acc; }, {}); - return { type: "data", result: await handler(handlerCtx) }; + let result = await handler(handlerCtx); + return result; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -1084,10 +1160,13 @@ describe("router dataStrategy", () => { }); return acc; }, {}); - return { type: "data", result: await callHandler(handlerCtx) }; + let result = m.shouldLoad + ? await callHandler(handlerCtx) + : t.router.state.loaderData[m.route.id]; + return result; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -1158,29 +1237,29 @@ describe("router dataStrategy", () => { ? [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.shouldLoad); return Promise.all( - matches.map(async (m) => { + matchesToLoad.map(async (m) => { return m.resolve(async (handler) => { - if (request.method !== "GET") { - // invalidate on actions - cache = {}; - return { type: "data", result: await handler() }; - } - let key = getCacheKey(m); if (key && cache[key]) { - return { type: "data", result: cache[key] }; + return cache[key]; } - let handlerResult = await handler(); - if (key) { - cache[key] = handlerResult; + let dsResult = await handler(); + if (key && request.method === "GET") { + cache[key] = dsResult; } - return { type: "data", result: handlerResult }; + return dsResult; }); }) - ); + ).then((results) => keyedResults(matchesToLoad, results)); }, }); diff --git a/packages/router/__tests__/utils/urlDataStrategy.ts b/packages/router/__tests__/utils/urlDataStrategy.ts index 5acf3687d8..5b70765348 100644 --- a/packages/router/__tests__/utils/urlDataStrategy.ts +++ b/packages/router/__tests__/utils/urlDataStrategy.ts @@ -1,13 +1,9 @@ -import type { - DataStrategyFunction, - DataStrategyFunctionArgs, -} from "@remix-run/router"; +import type { DataStrategyFunction } from "@remix-run/router"; import { invariant } from "./utils"; -export default async function urlDataStrategy({ - matches, -}: DataStrategyFunctionArgs): ReturnType { - return Promise.all( +const urlDataStrategy: DataStrategyFunction = async ({ matches }) => { + let results: Record = {}; + await Promise.all( matches.map((match) => match.resolve(async (handler) => { let response = await handler(); @@ -17,11 +13,14 @@ export default async function urlDataStrategy({ contentType === "application/x-www-form-urlencoded", "Invalid Response" ); - return { + results[match.route.id] = { type: "data", result: new URLSearchParams(await response.text()), }; }) ) ); -} + return results; +}; + +export default urlDataStrategy; diff --git a/packages/router/index.ts b/packages/router/index.ts index 5e6dcfe3dc..ef32be6f50 100644 --- a/packages/router/index.ts +++ b/packages/router/index.ts @@ -12,10 +12,10 @@ export type { DataStrategyFunction as unstable_DataStrategyFunction, DataStrategyFunctionArgs as unstable_DataStrategyFunctionArgs, DataStrategyMatch as unstable_DataStrategyMatch, + DataStrategyResult as unstable_DataStrategyResult, ErrorResponse, FormEncType, FormMethod, - HandlerResult as unstable_HandlerResult, HTMLFormMethod, JsonFunction, LazyRouteFunction, diff --git a/packages/router/router.ts b/packages/router/router.ts index 6261ead4cd..4691f002a8 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -22,7 +22,7 @@ import type { FormEncType, FormMethod, HTMLFormMethod, - HandlerResult, + DataStrategyResult, ImmutableRouteKey, MapRoutePropertiesFunction, MutationFormMethod, @@ -1758,11 +1758,13 @@ export function createRouter(init: RouterInit): Router { } else { let results = await callDataStrategy( "action", + state, request, [actionMatch], - matches + matches, + null ); - result = results[0]; + result = results[actionMatch.route.id]; if (request.signal.aborted) { return { shortCircuited: true }; @@ -2004,7 +2006,7 @@ export function createRouter(init: RouterInit): Router { let { loaderResults, fetcherResults } = await callLoadersAndMaybeResolveData( - state.matches, + state, matches, matchesToLoad, revalidatingFetchers, @@ -2027,16 +2029,20 @@ export function createRouter(init: RouterInit): Router { revalidatingFetchers.forEach((rf) => fetchControllers.delete(rf.key)); // If any loaders returned a redirect Response, start a new REPLACE navigation - let redirect = findRedirect([...loaderResults, ...fetcherResults]); + let redirect = findRedirect(loaderResults); if (redirect) { - if (redirect.idx >= matchesToLoad.length) { - // If this redirect came from a fetcher make sure we mark it in - // fetchRedirectIds so it doesn't get revalidated on the next set of - // loader executions - let fetcherKey = - revalidatingFetchers[redirect.idx - matchesToLoad.length].key; - fetchRedirectIds.add(fetcherKey); - } + await startRedirectNavigation(request, redirect.result, true, { + replace, + }); + return { shortCircuited: true }; + } + + redirect = findRedirect(fetcherResults); + if (redirect) { + // If this redirect came from a fetcher make sure we mark it in + // fetchRedirectIds so it doesn't get revalidated on the next set of + // loader executions + fetchRedirectIds.add(redirect.key); await startRedirectNavigation(request, redirect.result, true, { replace, }); @@ -2296,11 +2302,13 @@ export function createRouter(init: RouterInit): Router { let originatingLoadId = incrementingLoadId; let actionResults = await callDataStrategy( "action", + state, fetchRequest, [match], - requestMatches + requestMatches, + key ); - let actionResult = actionResults[0]; + let actionResult = actionResults[match.route.id]; if (fetchRequest.signal.aborted) { // We can delete this so long as we weren't aborted by our own fetcher @@ -2424,7 +2432,7 @@ export function createRouter(init: RouterInit): Router { let { loaderResults, fetcherResults } = await callLoadersAndMaybeResolveData( - state.matches, + state, matches, matchesToLoad, revalidatingFetchers, @@ -2444,16 +2452,21 @@ export function createRouter(init: RouterInit): Router { fetchControllers.delete(key); revalidatingFetchers.forEach((r) => fetchControllers.delete(r.key)); - let redirect = findRedirect([...loaderResults, ...fetcherResults]); + let redirect = findRedirect(loaderResults); if (redirect) { - if (redirect.idx >= matchesToLoad.length) { - // If this redirect came from a fetcher make sure we mark it in - // fetchRedirectIds so it doesn't get revalidated on the next set of - // loader executions - let fetcherKey = - revalidatingFetchers[redirect.idx - matchesToLoad.length].key; - fetchRedirectIds.add(fetcherKey); - } + return startRedirectNavigation( + revalidationRequest, + redirect.result, + false + ); + } + + redirect = findRedirect(fetcherResults); + if (redirect) { + // If this redirect came from a fetcher make sure we mark it in + // fetchRedirectIds so it doesn't get revalidated on the next set of + // loader executions + fetchRedirectIds.add(redirect.key); return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2464,7 +2477,7 @@ export function createRouter(init: RouterInit): Router { // Process and commit output from loaders let { loaderData, errors } = processLoaderData( state, - state.matches, + matches, matchesToLoad, loaderResults, undefined, @@ -2577,11 +2590,13 @@ export function createRouter(init: RouterInit): Router { let originatingLoadId = incrementingLoadId; let results = await callDataStrategy( "loader", + state, fetchRequest, [match], - matches + matches, + key ); - let result = results[0]; + let result = results[match.route.id]; // Deferred isn't supported for fetcher loads, await everything and treat it // as a normal load. resolveDeferredData will return undefined if this @@ -2775,102 +2790,123 @@ export function createRouter(init: RouterInit): Router { // pass around the manifest, mapRouteProperties, etc. async function callDataStrategy( type: "loader" | "action", + state: RouterState, request: Request, matchesToLoad: AgnosticDataRouteMatch[], - matches: AgnosticDataRouteMatch[] - ): Promise { + matches: AgnosticDataRouteMatch[], + fetcherKey: string | null + ): Promise> { + let results: Record; + let dataResults: Record = {}; try { - let results = await callDataStrategyImpl( + results = await callDataStrategyImpl( dataStrategyImpl, type, + state, request, matchesToLoad, matches, + fetcherKey, manifest, mapRouteProperties ); - - return await Promise.all( - results.map((result, i) => { - if (isRedirectHandlerResult(result)) { - let response = result.result as Response; - return { - type: ResultType.redirect, - response: normalizeRelativeRoutingRedirectResponse( - response, - request, - matchesToLoad[i].route.id, - matches, - basename, - future.v7_relativeSplatPath - ), - }; - } - - return convertHandlerResultToDataResult(result); - }) - ); } catch (e) { // If the outer dataStrategy method throws, just return the error for all // matches - and it'll naturally bubble to the root - return matchesToLoad.map(() => ({ - type: ResultType.error, - error: e, - })); + matchesToLoad.forEach((m) => { + dataResults[m.route.id] = { + type: ResultType.error, + error: e, + }; + }); + return dataResults; } + + for (let [routeId, result] of Object.entries(results)) { + if (isRedirectDataStrategyResultResult(result)) { + let response = result.result as Response; + dataResults[routeId] = { + type: ResultType.redirect, + response: normalizeRelativeRoutingRedirectResponse( + response, + request, + routeId, + matches, + basename, + future.v7_relativeSplatPath + ), + }; + } else { + dataResults[routeId] = await convertDataStrategyResultToDataResult( + result + ); + } + } + + return dataResults; } async function callLoadersAndMaybeResolveData( - currentMatches: AgnosticDataRouteMatch[], + state: RouterState, matches: AgnosticDataRouteMatch[], matchesToLoad: AgnosticDataRouteMatch[], fetchersToLoad: RevalidatingFetcher[], request: Request ) { - let [loaderResults, ...fetcherResults] = await Promise.all([ - matchesToLoad.length - ? callDataStrategy("loader", request, matchesToLoad, matches) - : [], - ...fetchersToLoad.map((f) => { + let currentMatches = state.matches; + + // Kick off loaders and fetchers in parallel + let loaderResultsPromise = callDataStrategy( + "loader", + state, + request, + matchesToLoad, + matches, + null + ); + + let fetcherResultsPromise = Promise.all( + fetchersToLoad.map(async (f) => { if (f.matches && f.match && f.controller) { - let fetcherRequest = createClientSideRequest( - init.history, - f.path, - f.controller.signal - ); - return callDataStrategy( + let results = await callDataStrategy( "loader", - fetcherRequest, + state, + createClientSideRequest(init.history, f.path, f.controller.signal), [f.match], - f.matches - ).then((r) => r[0]); + f.matches, + f.key + ); + let result = results[f.match.route.id]; + // Fetcher results are keyed by fetcher key from here on out, not routeId + return { [f.key]: result }; } else { - return Promise.resolve({ - type: ResultType.error, - error: getInternalRouterError(404, { - pathname: f.path, - }), + return Promise.resolve({ + [f.key]: { + type: ResultType.error, + error: getInternalRouterError(404, { + pathname: f.path, + }), + } as ErrorResult, }); } - }), - ]); + }) + ); + + let loaderResults = await loaderResultsPromise; + let fetcherResults = (await fetcherResultsPromise).reduce( + (acc, r) => Object.assign(acc, r), + {} + ); await Promise.all([ - resolveDeferredResults( - currentMatches, - matchesToLoad, + resolveNavigationDeferredResults( + matches, loaderResults, - loaderResults.map(() => request.signal), - false, - state.loaderData - ), - resolveDeferredResults( + request.signal, currentMatches, - fetchersToLoad.map((f) => f.match), - fetcherResults, - fetchersToLoad.map((f) => (f.controller ? f.controller.signal : null)), - true + state.loaderData ), + resolveFetcherDeferredResults(matches, fetcherResults, fetchersToLoad), ]); return { @@ -3720,9 +3756,9 @@ export function createStaticHandler( }; } catch (e) { // If the user threw/returned a Response in callLoaderOrAction for a - // `queryRoute` call, we throw the `HandlerResult` to bail out early + // `queryRoute` call, we throw the `DataStrategyResult` to bail out early // and then return or throw the raw Response here accordingly - if (isHandlerResult(e) && isResponse(e.result)) { + if (isDataStrategyResult(e) && isResponse(e.result)) { if (e.type === ResultType.error) { throw e.result; } @@ -3771,7 +3807,7 @@ export function createStaticHandler( requestContext, unstable_dataStrategy ); - result = results[0]; + result = results[actionMatch.route.id]; if (request.signal.aborted) { throwStaticHandlerAbortedError(request, isRouteRequest, future); @@ -3962,7 +3998,6 @@ export function createStaticHandler( let activeDeferreds = new Map(); let context = processRouteLoaderData( matches, - matchesToLoad, results, pendingActionResult, activeDeferreds, @@ -3999,27 +4034,34 @@ export function createStaticHandler( isRouteRequest: boolean, requestContext: unknown, unstable_dataStrategy: DataStrategyFunction | null - ): Promise { + ): Promise> { let results = await callDataStrategyImpl( unstable_dataStrategy || defaultDataStrategy, type, + null, request, matchesToLoad, matches, + null, manifest, mapRouteProperties, requestContext ); - return await Promise.all( - results.map((result, i) => { - if (isRedirectHandlerResult(result)) { + let dataResults: Record = {}; + await Promise.all( + matches.map(async (match) => { + if (!(match.route.id in results)) { + return; + } + let result = results[match.route.id]; + if (isRedirectDataStrategyResultResult(result)) { let response = result.result as Response; // Throw redirects and let the server handle them with an HTTP redirect throw normalizeRelativeRoutingRedirectResponse( response, request, - matchesToLoad[i].route.id, + match.route.id, matches, basename, future.v7_relativeSplatPath @@ -4031,9 +4073,11 @@ export function createStaticHandler( throw result; } - return convertHandlerResultToDataResult(result); + dataResults[match.route.id] = + await convertDataStrategyResultToDataResult(result); }) ); + return dataResults; } return { @@ -4724,77 +4768,91 @@ async function loadLazyRouteModule( } // Default implementation of `dataStrategy` which fetches all loaders in parallel -function defaultDataStrategy( - opts: DataStrategyFunctionArgs -): ReturnType { - return Promise.all(opts.matches.map((m) => m.resolve())); +async function defaultDataStrategy({ + matches, +}: DataStrategyFunctionArgs): ReturnType { + let matchesToLoad = matches.filter((m) => m.shouldLoad); + let results = await Promise.all(matchesToLoad.map((m) => m.resolve())); + return results.reduce( + (acc, result, i) => + Object.assign(acc, { [matchesToLoad[i].route.id]: result }), + {} + ); } async function callDataStrategyImpl( dataStrategyImpl: DataStrategyFunction, type: "loader" | "action", + state: RouterState | null, request: Request, matchesToLoad: AgnosticDataRouteMatch[], matches: AgnosticDataRouteMatch[], + fetcherKey: string | null, manifest: RouteManifest, mapRouteProperties: MapRoutePropertiesFunction, requestContext?: unknown -): Promise { - let routeIdsToLoad = matchesToLoad.reduce( - (acc, m) => acc.add(m.route.id), - new Set() +): Promise> { + let loadRouteDefinitionsPromises = matches.map((m) => + m.route.lazy + ? loadLazyRouteModule(m.route, mapRouteProperties, manifest) + : undefined ); - let loadedMatches = new Set(); + + let dsMatches = matches.map((match, i) => { + let loadRoutePromise = loadRouteDefinitionsPromises[i]; + let shouldLoad = matchesToLoad.some((m) => m.route.id === match.route.id); + // `resolve` encapsulates route.lazy(), executing the loader/action, + // and mapping return values/thrown errors to a `DataStrategyResult`. Users + // can pass a callback to take fine-grained control over the execution + // of the loader/action + let resolve: DataStrategyMatch["resolve"] = async (handlerOverride) => { + if ( + handlerOverride && + request.method === "GET" && + (match.route.lazy || match.route.loader) + ) { + shouldLoad = true; + } + return shouldLoad + ? callLoaderOrAction( + type, + request, + match, + loadRoutePromise, + handlerOverride, + requestContext + ) + : Promise.resolve({ type: ResultType.data, result: undefined }); + }; + + return { + ...match, + shouldLoad, + resolve, + }; + }); // Send all matches here to allow for a middleware-type implementation. // handler will be a no-op for unneeded routes and we filter those results // back out below. let results = await dataStrategyImpl({ - matches: matches.map((match) => { - let shouldLoad = routeIdsToLoad.has(match.route.id); - // `resolve` encapsulates the route.lazy, executing the - // loader/action, and mapping return values/thrown errors to a - // HandlerResult. Users can pass a callback to take fine-grained control - // over the execution of the loader/action - let resolve: DataStrategyMatch["resolve"] = (handlerOverride) => { - loadedMatches.add(match.route.id); - return shouldLoad - ? callLoaderOrAction( - type, - request, - match, - manifest, - mapRouteProperties, - handlerOverride, - requestContext - ) - : Promise.resolve({ type: ResultType.data, result: undefined }); - }; - - return { - ...match, - shouldLoad, - resolve, - }; - }), + matches: dsMatches, request, params: matches[0].params, + fetcherKey, context: requestContext, }); - // Throw if any loadRoute implementations not called since they are what - // ensures a route is fully loaded - matches.forEach((m) => - invariant( - loadedMatches.has(m.route.id), - `\`match.resolve()\` was not called for route id "${m.route.id}". ` + - "You must call `match.resolve()` on every match passed to " + - "`dataStrategy` to ensure all routes are properly loaded." - ) - ); + // Wait for all routes to load here but 'swallow the error since we want + // it to bubble up from the `await loadRoutePromise` in `callLoaderOrAction` - + // called from `match.resolve()` + try { + await Promise.all(loadRouteDefinitionsPromises); + } catch (e) { + // No-op + } - // Filter out any middleware-only matches for which we didn't need to run handlers - return results.filter((_, i) => routeIdsToLoad.has(matches[i].route.id)); + return results; } // Default logic for calling a loader/action is the user has no specified a dataStrategy @@ -4802,22 +4860,21 @@ async function callLoaderOrAction( type: "loader" | "action", request: Request, match: AgnosticDataRouteMatch, - manifest: RouteManifest, - mapRouteProperties: MapRoutePropertiesFunction, + loadRoutePromise: Promise | undefined, handlerOverride: Parameters[0], staticContext?: unknown -): Promise { - let result: HandlerResult; +): Promise { + let result: DataStrategyResult; let onReject: (() => void) | undefined; let runHandler = ( handler: AgnosticRouteObject["loader"] | AgnosticRouteObject["action"] - ): Promise => { + ): Promise => { // Setup a promise we can race against so that abort signals short circuit let reject: () => void; - // This will never resolve so safe to type it as Promise to + // This will never resolve so safe to type it as Promise to // satisfy the function return value - let abortPromise = new Promise((_, r) => (reject = r)); + let abortPromise = new Promise((_, r) => (reject = r)); onReject = () => reject(); request.signal.addEventListener("abort", onReject); @@ -4840,19 +4897,16 @@ async function callLoaderOrAction( ); }; - let handlerPromise: Promise; - if (handlerOverride) { - handlerPromise = handlerOverride((ctx: unknown) => actualHandler(ctx)); - } else { - handlerPromise = (async () => { - try { - let val = await actualHandler(); - return { type: "data", result: val }; - } catch (e) { - return { type: "error", result: e }; - } - })(); - } + let handlerPromise: Promise = (async () => { + try { + let val = await (handlerOverride + ? handlerOverride((ctx: unknown) => actualHandler(ctx)) + : actualHandler()); + return { type: "data", result: val }; + } catch (e) { + return { type: "error", result: e }; + } + })(); return Promise.race([handlerPromise, abortPromise]); }; @@ -4860,7 +4914,8 @@ async function callLoaderOrAction( try { let handler = match.route[type]; - if (match.route.lazy) { + // If we have a route.lazy promise, await that first + if (loadRoutePromise) { if (handler) { // Run statically defined handler in parallel with lazy() let handlerError; @@ -4871,7 +4926,7 @@ async function callLoaderOrAction( runHandler(handler).catch((e) => { handlerError = e; }), - loadLazyRouteModule(match.route, mapRouteProperties, manifest), + loadRoutePromise, ]); if (handlerError !== undefined) { throw handlerError; @@ -4879,7 +4934,7 @@ async function callLoaderOrAction( result = value!; } else { // Load lazy route module, then run any returned handler - await loadLazyRouteModule(match.route, mapRouteProperties, manifest); + await loadRoutePromise; handler = match.route[type]; if (handler) { @@ -4919,7 +4974,7 @@ async function callLoaderOrAction( ); } catch (e) { // We should already be catching and converting normal handler executions to - // HandlerResults and returning them, so anything that throws here is an + // DataStrategyResults and returning them, so anything that throws here is an // unexpected error we still need to wrap return { type: ResultType.error, result: e }; } finally { @@ -4931,10 +4986,10 @@ async function callLoaderOrAction( return result; } -async function convertHandlerResultToDataResult( - handlerResult: HandlerResult +async function convertDataStrategyResultToDataResult( + dataStrategyResult: DataStrategyResult ): Promise { - let { result, type } = handlerResult; + let { result, type } = dataStrategyResult; if (isResponse(result)) { let data: any; @@ -5136,8 +5191,7 @@ function convertSearchParamsToFormData( function processRouteLoaderData( matches: AgnosticDataRouteMatch[], - matchesToLoad: AgnosticDataRouteMatch[], - results: DataResult[], + results: Record, pendingActionResult: PendingActionResult | undefined, activeDeferreds: Map, skipLoaderErrorBubbling: boolean @@ -5159,8 +5213,12 @@ function processRouteLoaderData( : undefined; // Process loader results into state.loaderData/state.errors - results.forEach((result, index) => { - let id = matchesToLoad[index].route.id; + matches.forEach((match) => { + if (!(match.route.id in results)) { + return; + } + let id = match.route.id; + let result = results[id]; invariant( !isRedirectResult(result), "Cannot handle redirect results in processLoaderData" @@ -5253,10 +5311,10 @@ function processLoaderData( state: RouterState, matches: AgnosticDataRouteMatch[], matchesToLoad: AgnosticDataRouteMatch[], - results: DataResult[], + results: Record, pendingActionResult: PendingActionResult | undefined, revalidatingFetchers: RevalidatingFetcher[], - fetcherResults: DataResult[], + fetcherResults: Record, activeDeferreds: Map ): { loaderData: RouterState["loaderData"]; @@ -5264,7 +5322,6 @@ function processLoaderData( } { let { loaderData, errors } = processRouteLoaderData( matches, - matchesToLoad, results, pendingActionResult, activeDeferreds, @@ -5272,18 +5329,15 @@ function processLoaderData( ); // Process results from our revalidating fetchers - for (let index = 0; index < revalidatingFetchers.length; index++) { - let { key, match, controller } = revalidatingFetchers[index]; - invariant( - fetcherResults !== undefined && fetcherResults[index] !== undefined, - "Did not find corresponding fetcher result" - ); - let result = fetcherResults[index]; + revalidatingFetchers.forEach((rf) => { + let { key, match, controller } = rf; + let result = fetcherResults[key]; + invariant(result, "Did not find corresponding fetcher result"); // Process fetcher non-redirect errors if (controller && controller.signal.aborted) { // Nothing to do for aborted fetchers - continue; + return; } else if (isErrorResult(result)) { let boundaryMatch = findNearestBoundary(state.matches, match?.route.id); if (!(errors && errors[boundaryMatch.route.id])) { @@ -5305,7 +5359,7 @@ function processLoaderData( let doneFetcher = getDoneFetcher(result.data); state.fetchers.set(key, doneFetcher); } - } + }); return { loaderData, errors }; } @@ -5463,12 +5517,13 @@ function getInternalRouterError( // Find any returned redirect errors, starting from the lowest match function findRedirect( - results: DataResult[] -): { result: RedirectResult; idx: number } | undefined { - for (let i = results.length - 1; i >= 0; i--) { - let result = results[i]; + results: Record +): { key: string; result: RedirectResult } | undefined { + let entries = Object.entries(results); + for (let i = entries.length - 1; i >= 0; i--) { + let [key, result] = entries[i]; if (isRedirectResult(result)) { - return { result, idx: i }; + return { key, result }; } } } @@ -5503,7 +5558,7 @@ function isPromise(val: unknown): val is Promise { return typeof val === "object" && val != null && "then" in val; } -function isHandlerResult(result: unknown): result is HandlerResult { +function isDataStrategyResult(result: unknown): result is DataStrategyResult { return ( result != null && typeof result === "object" && @@ -5513,7 +5568,7 @@ function isHandlerResult(result: unknown): result is HandlerResult { ); } -function isRedirectHandlerResult(result: HandlerResult) { +function isRedirectDataStrategyResultResult(result: DataStrategyResult) { return ( isResponse(result.result) && redirectStatusCodes.has(result.result.status) ); @@ -5586,17 +5641,17 @@ function isMutationMethod( return validMutationMethods.has(method.toLowerCase() as MutationFormMethod); } -async function resolveDeferredResults( +async function resolveNavigationDeferredResults( + matches: (AgnosticDataRouteMatch | null)[], + results: Record, + signal: AbortSignal, currentMatches: AgnosticDataRouteMatch[], - matchesToLoad: (AgnosticDataRouteMatch | null)[], - results: DataResult[], - signals: (AbortSignal | null)[], - isFetcher: boolean, - currentLoaderData?: RouteData + currentLoaderData: RouteData ) { - for (let index = 0; index < results.length; index++) { - let result = results[index]; - let match = matchesToLoad[index]; + let entries = Object.entries(results); + for (let index = 0; index < entries.length; index++) { + let [routeId, result] = entries[index]; + let match = matches.find((m) => m?.route.id === routeId); // If we don't have a match, then we can have a deferred result to do // anything with. This is for revalidating fetchers where the route was // removed during HMR @@ -5612,24 +5667,54 @@ async function resolveDeferredResults( !isNewRouteInstance(currentMatch, match) && (currentLoaderData && currentLoaderData[match.route.id]) !== undefined; - if (isDeferredResult(result) && (isFetcher || isRevalidatingLoader)) { + if (isDeferredResult(result) && isRevalidatingLoader) { // Note: we do not have to touch activeDeferreds here since we race them // against the signal in resolveDeferredData and they'll get aborted // there if needed - let signal = signals[index]; - invariant( - signal, - "Expected an AbortSignal for revalidating fetcher deferred result" - ); - await resolveDeferredData(result, signal, isFetcher).then((result) => { + await resolveDeferredData(result, signal, false).then((result) => { if (result) { - results[index] = result || results[index]; + results[routeId] = result; } }); } } } +async function resolveFetcherDeferredResults( + matches: (AgnosticDataRouteMatch | null)[], + results: Record, + revalidatingFetchers: RevalidatingFetcher[] +) { + for (let index = 0; index < revalidatingFetchers.length; index++) { + let { key, routeId, controller } = revalidatingFetchers[index]; + let result = results[key]; + let match = matches.find((m) => m?.route.id === routeId); + // If we don't have a match, then we can have a deferred result to do + // anything with. This is for revalidating fetchers where the route was + // removed during HMR + if (!match) { + continue; + } + + if (isDeferredResult(result)) { + // Note: we do not have to touch activeDeferreds here since we race them + // against the signal in resolveDeferredData and they'll get aborted + // there if needed + invariant( + controller, + "Expected an AbortController for revalidating fetcher deferred result" + ); + await resolveDeferredData(result, controller.signal, true).then( + (result) => { + if (result) { + results[key] = result; + } + } + ); + } + } +} + async function resolveDeferredData( result: DeferredResult, signal: AbortSignal, diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 8b9df61f10..c6586aee23 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -63,14 +63,6 @@ export type DataResult = | RedirectResult | ErrorResult; -/** - * Result from a loader or action called via dataStrategy - */ -export interface HandlerResult { - type: "data" | "error"; - result: unknown; // data, Error, Response, DeferredData, DataWithResponseInit -} - type LowerCaseFormMethod = "get" | "post" | "put" | "patch" | "delete"; type UpperCaseFormMethod = Uppercase; @@ -241,17 +233,26 @@ export interface DataStrategyMatch resolve: ( handlerOverride?: ( handler: (ctx?: unknown) => DataFunctionReturnValue - ) => Promise - ) => Promise; + ) => DataFunctionReturnValue + ) => Promise; } export interface DataStrategyFunctionArgs extends DataFunctionArgs { matches: DataStrategyMatch[]; + fetcherKey: string | null; +} + +/** + * Result from a loader or action called via dataStrategy + */ +export interface DataStrategyResult { + type: "data" | "error"; + result: unknown; // data, Error, Response, DeferredData, DataWithResponseInit } export interface DataStrategyFunction { - (args: DataStrategyFunctionArgs): Promise; + (args: DataStrategyFunctionArgs): Promise>; } export interface AgnosticPatchRoutesOnNavigationFunction<