Update logic for data->Response conversion to use Response.json (#14181)

Author: brophdawg11

.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`)

.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

integration/resource-routes-test.ts

@@ -243,15 +243,15 @@ 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 }) => {
     let app = new PlaywrightFixture(appFixture, page);
     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 ({

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", () => {

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<D>(
   data: DataWithResponseInit<D>,
 ): 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<D>(
@@ -6601,16 +6596,6 @@ function dataWithResponseInitToErrorResponse<D>(
   );
 }
 
-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 &&