From 1a232bd94f91acc4cac5c6500eb8ed4dd7e81d76 Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Tue, 14 Nov 2023 10:58:50 -0800 Subject: [PATCH 1/3] feat: unwrapResponse for custom response handling --- .changeset/unwrap-response.md | 5 +++++ package.json | 2 +- packages/router/router.ts | 28 +++++++++++++++++++++++----- 3 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 .changeset/unwrap-response.md diff --git a/.changeset/unwrap-response.md b/.changeset/unwrap-response.md new file mode 100644 index 0000000000..187f338cda --- /dev/null +++ b/.changeset/unwrap-response.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": minor +--- + +Add `unwrapResponse` option to allow unwrapping of more complex data formats than just json / text. diff --git a/package.json b/package.json index ce06a4af7e..85c4525fe2 100644 --- a/package.json +++ b/package.json @@ -110,7 +110,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "49.3 kB" + "none": "49.4 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "13.9 kB" diff --git a/packages/router/router.ts b/packages/router/router.ts index c14c4814da..2dc25a1e67 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -348,6 +348,8 @@ export interface FutureConfig { v7_prependBasename: boolean; } +export type UnwrapResponseFunction = (response: Response) => Promise; + /** * Initialization options for createRouter */ @@ -362,6 +364,7 @@ export interface RouterInit { mapRouteProperties?: MapRoutePropertiesFunction; future?: Partial; hydrationData?: HydrationState; + unwrapResponse?: UnwrapResponseFunction; window?: Window; } @@ -735,6 +738,7 @@ export function createRouter(init: RouterInit): Router { typeof routerWindow.document !== "undefined" && typeof routerWindow.document.createElement !== "undefined"; const isServer = !isBrowser; + const unwrapResponse = init.unwrapResponse; invariant( init.routes.length > 0, @@ -1545,7 +1549,8 @@ export function createRouter(init: RouterInit): Router { matches, manifest, mapRouteProperties, - basename + basename, + unwrapResponse ); if (request.signal.aborted) { @@ -1929,7 +1934,8 @@ export function createRouter(init: RouterInit): Router { requestMatches, manifest, mapRouteProperties, - basename + basename, + unwrapResponse ); if (fetchRequest.signal.aborted) { @@ -2171,7 +2177,8 @@ export function createRouter(init: RouterInit): Router { matches, manifest, mapRouteProperties, - basename + basename, + unwrapResponse ); // Deferred isn't supported for fetcher loads, await everything and treat it @@ -2367,7 +2374,8 @@ export function createRouter(init: RouterInit): Router { matches, manifest, mapRouteProperties, - basename + basename, + unwrapResponse ) ), ...fetchersToLoad.map((f) => { @@ -2379,7 +2387,8 @@ export function createRouter(init: RouterInit): Router { f.matches, manifest, mapRouteProperties, - basename + basename, + unwrapResponse ); } else { let error: ErrorResult = { @@ -2769,6 +2778,7 @@ export interface CreateStaticHandlerOptions { */ detectErrorBoundary?: DetectErrorBoundaryFunction; mapRouteProperties?: MapRoutePropertiesFunction; + unwrapResponse?: UnwrapResponseFunction; } export function createStaticHandler( @@ -2779,6 +2789,7 @@ export function createStaticHandler( routes.length > 0, "You must provide a non-empty routes array to createStaticHandler" ); + let unwrapResponse = opts?.unwrapResponse; let manifest: RouteManifest = {}; let basename = (opts ? opts.basename : null) || "/"; @@ -3056,6 +3067,7 @@ export function createStaticHandler( manifest, mapRouteProperties, basename, + unwrapResponse, { isStaticRequest: true, isRouteRequest, requestContext } ); @@ -3224,6 +3236,7 @@ export function createStaticHandler( manifest, mapRouteProperties, basename, + unwrapResponse, { isStaticRequest: true, isRouteRequest, requestContext } ) ), @@ -3824,6 +3837,7 @@ async function callLoaderOrAction( manifest: RouteManifest, mapRouteProperties: MapRoutePropertiesFunction, basename: string, + unwrapResponse: UnwrapResponseFunction | undefined, opts: { isStaticRequest?: boolean; isRouteRequest?: boolean; @@ -3984,6 +3998,9 @@ async function callLoaderOrAction( } let data: any; + if (unwrapResponse) { + data = unwrapResponse(result); + } else { let contentType = result.headers.get("Content-Type"); // Check between word boundaries instead of startsWith() due to the last // paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type @@ -3991,6 +4008,7 @@ async function callLoaderOrAction( data = await result.json(); } else { data = await result.text(); + } } if (resultType === ResultType.error) { From 0536008e365da061b823b9b1d61818b558114f8a Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 20 Nov 2023 15:41:03 -0500 Subject: [PATCH 2/3] Minor updates + unit tests --- packages/router/__tests__/router-test.ts | 114 ++++++++++++++++++ .../__tests__/utils/data-router-setup.ts | 20 +-- packages/router/router.ts | 38 +++--- 3 files changed, 143 insertions(+), 29 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index bbe7aeced5..e29dd51458 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -2714,4 +2714,118 @@ describe("a router", () => { expect(B.loaders.tasks.signal.aborted).toBe(true); }); }); + + describe("unwrapResponse", () => { + it("should unwrap json and text by default", async () => { + let t = setup({ + routes: [ + { + path: "/", + }, + { + id: "json", + path: "/test", + loader: true, + children: [ + { + id: "text", + index: true, + loader: true, + }, + ], + }, + ], + }); + + let A = await t.navigate("/test"); + await A.loaders.json.resolve( + new Response(JSON.stringify({ message: "hello json" }), { + headers: { + "Content-Type": "application/json", + }, + }) + ); + await A.loaders.text.resolve(new Response("hello text")); + + expect(t.router.state.loaderData).toEqual({ + json: { message: "hello json" }, + text: "hello text", + }); + }); + + it("should allow custom implementations to be provided", async () => { + let t = setup({ + routes: [ + { + path: "/", + }, + { + id: "test", + path: "/test", + loader: true, + }, + ], + async unwrapResponse(response) { + if ( + response.headers.get("Content-Type") === + "application/x-www-form-urlencoded" + ) { + let text = await response.text(); + return new URLSearchParams(text); + } + throw new Error("Unknown Content-Type"); + }, + }); + + let A = await t.navigate("/test"); + await A.loaders.test.resolve( + new Response(new URLSearchParams({ a: "1", b: "2" }).toString(), { + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + }) + ); + + expect(t.router.state.loaderData.test).toBeInstanceOf(URLSearchParams); + expect(t.router.state.loaderData.test.toString()).toBe("a=1&b=2"); + }); + + it("handles errors thrown from unwrapResponse at the proper boundary", async () => { + let t = setup({ + routes: [ + { + path: "/", + }, + { + path: "/parent", + children: [ + { + id: "child", + path: "child", + hasErrorBoundary: true, + children: [ + { + id: "test", + index: true, + loader: true, + }, + ], + }, + ], + }, + ], + async unwrapResponse(response) { + throw new Error("Unable to unwrap response"); + }, + }); + + let A = await t.navigate("/parent/child"); + await A.loaders.test.resolve(new Response("hello world")); + + expect(t.router.state.loaderData.test).toBeUndefined(); + expect(t.router.state.errors.child.message).toBe( + "Unable to unwrap response" + ); + }); + }); }); diff --git a/packages/router/__tests__/utils/data-router-setup.ts b/packages/router/__tests__/utils/data-router-setup.ts index 94bcee48fa..66170c2e81 100644 --- a/packages/router/__tests__/utils/data-router-setup.ts +++ b/packages/router/__tests__/utils/data-router-setup.ts @@ -4,11 +4,10 @@ import type { AgnosticRouteMatch, Fetcher, RouterFetchOptions, - HydrationState, InitialEntry, Router, RouterNavigateOptions, - FutureConfig, + RouterInit, } from "../../index"; import { createMemoryHistory, @@ -138,11 +137,8 @@ export const TASK_ROUTES: TestRouteObject[] = [ type SetupOpts = { routes: TestRouteObject[]; - basename?: string; initialEntries?: InitialEntry[]; initialIndex?: number; - hydrationData?: HydrationState; - future?: FutureConfig; }; // We use a slightly modified version of createDeferred here that includes the @@ -172,12 +168,10 @@ export function createDeferred() { export function setup({ routes, - basename, initialEntries, initialIndex, - hydrationData, - future, -}: SetupOpts) { + ...routerInit +}: Omit & SetupOpts) { let guid = 0; // Global "active" helpers, keyed by navType:guid:loaderOrAction:routeId. // For example, the first navigation for /parent/foo would generate: @@ -299,9 +293,9 @@ export function setup({ // jsdom is making more and more properties non-configurable, so we inject // our own jest-friendly window. let testWindow = { - ...window, + ...(routerInit.window || window), location: { - ...window.location, + ...(routerInit.window || window).location, assign: jest.fn(), replace: jest.fn(), }, @@ -312,11 +306,9 @@ export function setup({ jest.spyOn(history, "push"); jest.spyOn(history, "replace"); currentRouter = createRouter({ - basename, + ...routerInit, history, routes: enhanceRoutes(routes), - hydrationData, - future, window: testWindow, }).initialize(); diff --git a/packages/router/router.ts b/packages/router/router.ts index 2dc25a1e67..afb6178224 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -716,6 +716,17 @@ const defaultMapRouteProperties: MapRoutePropertiesFunction = (route) => ({ hasErrorBoundary: Boolean(route.hasErrorBoundary), }); +const defaultUnwrapResponse: UnwrapResponseFunction = (response) => { + let contentType = response.headers.get("Content-Type"); + // Check between word boundaries instead of startsWith() due to the last + // paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type + if (contentType && /\bapplication\/json\b/.test(contentType)) { + return response.json(); + } else { + return response.text(); + } +}; + const TRANSITIONS_STORAGE_KEY = "remix-router-transitions"; //#endregion @@ -738,7 +749,7 @@ export function createRouter(init: RouterInit): Router { typeof routerWindow.document !== "undefined" && typeof routerWindow.document.createElement !== "undefined"; const isServer = !isBrowser; - const unwrapResponse = init.unwrapResponse; + const unwrapResponse = init.unwrapResponse || defaultUnwrapResponse; invariant( init.routes.length > 0, @@ -2789,7 +2800,7 @@ export function createStaticHandler( routes.length > 0, "You must provide a non-empty routes array to createStaticHandler" ); - let unwrapResponse = opts?.unwrapResponse; + let unwrapResponse = opts?.unwrapResponse || defaultUnwrapResponse; let manifest: RouteManifest = {}; let basename = (opts ? opts.basename : null) || "/"; @@ -3837,7 +3848,7 @@ async function callLoaderOrAction( manifest: RouteManifest, mapRouteProperties: MapRoutePropertiesFunction, basename: string, - unwrapResponse: UnwrapResponseFunction | undefined, + unwrapResponse: UnwrapResponseFunction, opts: { isStaticRequest?: boolean; isRouteRequest?: boolean; @@ -3997,18 +4008,15 @@ async function callLoaderOrAction( throw queryRouteResponse; } - let data: any; - if (unwrapResponse) { - data = unwrapResponse(result); - } else { - let contentType = result.headers.get("Content-Type"); - // Check between word boundaries instead of startsWith() due to the last - // paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type - if (contentType && /\bapplication\/json\b/.test(contentType)) { - data = await result.json(); - } else { - data = await result.text(); - } + let data: unknown; + try { + data = await unwrapResponse(result); + } catch (e) { + resultType = ResultType.error; + return { + type: ResultType.error, + error: e, + }; } if (resultType === ResultType.error) { From 646769a3e0879482d9e9cde4e274e585e14b4c54 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 20 Nov 2023 15:43:57 -0500 Subject: [PATCH 3/3] Bump bundle --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 85c4525fe2..39d2264a85 100644 --- a/package.json +++ b/package.json @@ -110,7 +110,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "49.4 kB" + "none": "49.5 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "13.9 kB"