diff --git a/.changeset/many-beers-notice.md b/.changeset/many-beers-notice.md new file mode 100644 index 0000000000..d79e08b9b6 --- /dev/null +++ b/.changeset/many-beers-notice.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +[REMOVE] Update data -> Response conversion (update changelog with latest from `rotten-steaks-perform.md`) diff --git a/.changeset/rotten-steaks-perform.md b/.changeset/rotten-steaks-perform.md index 36834b9eb8..ff80404e3c 100644 --- a/.changeset/rotten-steaks-perform.md +++ b/.changeset/rotten-steaks-perform.md @@ -2,4 +2,4 @@ "react-router": patch --- -Properly handle data() values returned or thrown from resource routes and return corresponding responses with the data, status, and headers +Properly convert returned/thrown `data()` values to `Response` instances via `Response.json()` in middleware and resource routes diff --git a/integration/resource-routes-test.ts b/integration/resource-routes-test.ts index 0dee85408d..0718fe1c1c 100644 --- a/integration/resource-routes-test.ts +++ b/integration/resource-routes-test.ts @@ -243,7 +243,7 @@ test.describe("loader in an app", async () => { let res = await app.goto("/return-data"); expect(res.status()).toBe(207); expect(res.headers()["x-foo"]).toBe("Bar"); - expect(await res.text()).toEqual("Partial"); + expect(await res.json()).toEqual("Partial"); }); test("should handle data() thrown from resource routes", async ({ page }) => { @@ -251,7 +251,7 @@ test.describe("loader in an app", async () => { let res = await app.goto("/throw-data"); expect(res.status()).toBe(207); expect(res.headers()["x-foo"]).toBe("Bar"); - expect(await res.text()).toEqual("Partial"); + expect(await res.json()).toEqual("Partial"); }); test("should handle data() returned from resource routes through middleware", async ({ @@ -261,7 +261,7 @@ test.describe("loader in an app", async () => { let res = await app.goto("/return-data-through-middleware"); expect(res.status()).toBe(207); expect(res.headers()["x-foo"]).toBe("Bar"); - expect(await res.text()).toEqual("Partial"); + expect(await res.json()).toEqual("Partial"); }); test("should handle data() thrown from resource routes through middleware", async ({ @@ -271,7 +271,7 @@ test.describe("loader in an app", async () => { let res = await app.goto("/throw-data-through-middleware"); expect(res.status()).toBe(207); expect(res.headers()["x-foo"]).toBe("Bar"); - expect(await res.text()).toEqual("Partial"); + expect(await res.json()).toEqual("Partial"); }); test("should convert strings returned from resource routes to text responses", async ({ diff --git a/packages/react-router/__tests__/router/context-middleware-test.ts b/packages/react-router/__tests__/router/context-middleware-test.ts index f254c735c0..0825d61a72 100644 --- a/packages/react-router/__tests__/router/context-middleware-test.ts +++ b/packages/react-router/__tests__/router/context-middleware-test.ts @@ -1716,7 +1716,7 @@ describe("context/middleware", () => { respondWithJson(await q(request)), })) as Response; expect(res.status).toBe(404); - await expect(res.text()).resolves.toEqual("not found"); + await expect(res.json()).resolves.toEqual("not found"); }); it("propagates a thrown data() response if next isn't called", async () => { @@ -1800,7 +1800,7 @@ describe("context/middleware", () => { respondWithJson(await q(request)), })) as Response; expect(res.status).toBe(404); - await expect(res.text()).resolves.toEqual("not found"); + await expect(res.json()).resolves.toEqual("not found"); }); it("propagates a thrown data() response if next is called", async () => { @@ -2775,7 +2775,7 @@ describe("context/middleware", () => { unstable_generateMiddlewareResponse: (q) => q(request), })) as Response; expect(res.status).toBe(404); - await expect(res.text()).resolves.toEqual("not found"); + await expect(res.json()).resolves.toEqual("not found"); }); it("propagates a thrown data() response if next isn't called", async () => { @@ -2808,7 +2808,7 @@ describe("context/middleware", () => { unstable_generateMiddlewareResponse: async (q) => q(request), })) as Response; expect(res.status).toBe(404); - await expect(res.text()).resolves.toEqual("not found"); + await expect(res.json()).resolves.toEqual("not found"); }); it("propagates a returned data() response if next is called", async () => { @@ -2842,7 +2842,7 @@ describe("context/middleware", () => { unstable_generateMiddlewareResponse: (q) => q(request), })) as Response; expect(res.status).toBe(404); - await expect(res.text()).resolves.toEqual("not found"); + await expect(res.json()).resolves.toEqual("not found"); }); it("propagates a thrown data() response if next is called", async () => { @@ -2876,7 +2876,7 @@ describe("context/middleware", () => { unstable_generateMiddlewareResponse: async (q) => q(request), })) as Response; expect(res.status).toBe(404); - await expect(res.text()).resolves.toEqual("not found"); + await expect(res.json()).resolves.toEqual("not found"); }); describe("ordering", () => { diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 3449a2a789..895cc9b8d0 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -3651,6 +3651,11 @@ export function createStaticHandler( return error; } + if (isDataWithResponseInit(error)) { + // Convert thrown data() values to ErrorResponses for the UI + error = dataWithResponseInitToErrorResponse(error); + } + if (renderedStaticContext) { // We rendered an HTML response and caught an error going back up // the middleware chain - render again with an updated context @@ -3850,8 +3855,9 @@ export function createStaticHandler( return res; }, (error) => { - if (isRouteErrorResponse(error)) { - return Promise.resolve(errorResponseToResponse(error)); + if (isDataWithResponseInit(error)) { + // Convert thrown data() values to Responses for resource routes + return Promise.resolve(dataWithResponseInitToResponse(error)); } if (isResponse(error)) { return Promise.resolve(error); @@ -5511,11 +5517,7 @@ async function callServerRouteMiddleware( nextResult = result; return nextResult; } catch (e) { - nextResult = await errorHandler( - // Convert thrown data() values to ErrorResponses - isDataWithResponseInit(e) ? dataWithResponseInitToErrorResponse(e) : e, - routeId, - ); + nextResult = await errorHandler(e, routeId); return nextResult; } }; @@ -5550,11 +5552,7 @@ async function callServerRouteMiddleware( return nextResult; } } catch (e) { - let response = await errorHandler( - // Convert thrown data() values to ErrorResponses - isDataWithResponseInit(e) ? dataWithResponseInitToErrorResponse(e) : e, - routeId, - ); + let response = await errorHandler(e, routeId); return response; } } @@ -6585,10 +6583,7 @@ function isHashChangeOnly(a: Location, b: Location): boolean { function dataWithResponseInitToResponse( data: DataWithResponseInit, ): Response { - return new Response( - typeof data.data === "string" ? data.data : JSON.stringify(data.data), - data.init || undefined, - ); + return Response.json(data.data, data.init ?? undefined); } function dataWithResponseInitToErrorResponse( @@ -6601,16 +6596,6 @@ function dataWithResponseInitToErrorResponse( ); } -function errorResponseToResponse(error: ErrorResponse): Response { - return new Response( - typeof error.data === "string" ? error.data : JSON.stringify(error.data), - { - status: error.status, - statusText: error.statusText, - }, - ); -} - function isDataStrategyResult(result: unknown): result is DataStrategyResult { return ( result != null &&