Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/many-beers-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

[REMOVE] Update data -> Response conversion (update changelog with latest from `rotten-steaks-perform.md`)
2 changes: 1 addition & 1 deletion .changeset/rotten-steaks-perform.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 4 additions & 4 deletions integration/resource-routes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ({
Expand All @@ -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 ({
Expand All @@ -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 ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
37 changes: 11 additions & 26 deletions packages/react-router/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
};
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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>(
Expand All @@ -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 &&
Expand Down
Loading