From 96259d71d5981c4f7348ecea32b8a6c3485e536e Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 23 Jan 2025 14:04:32 -0500 Subject: [PATCH] Loosen restrictions on resource route --- .changeset/kind-hats-greet.md | 8 +++++ integration/resource-routes-test.ts | 35 ++++++++++++++----- .../react-router/lib/server-runtime/server.ts | 13 ++++--- 3 files changed, 43 insertions(+), 13 deletions(-) create mode 100644 .changeset/kind-hats-greet.md diff --git a/.changeset/kind-hats-greet.md b/.changeset/kind-hats-greet.md new file mode 100644 index 0000000000..7a707c620a --- /dev/null +++ b/.changeset/kind-hats-greet.md @@ -0,0 +1,8 @@ +--- +"react-router": patch +--- + +Stop erroring on resource routes that return raw strings/objects and instead serialize them as `text/plain` or `application/json` responses + +- This only applies when accessed as a resource route without the `.data` extension +- When accessed from a Single Fetch `.data` request, they will still be encoded via `turbo-stream` diff --git a/integration/resource-routes-test.ts b/integration/resource-routes-test.ts index 527813b2c9..28eb41b875 100644 --- a/integration/resource-routes-test.ts +++ b/integration/resource-routes-test.ts @@ -61,7 +61,7 @@ test.describe("loader in an app", async () => { export default () =>
You made it!
`, "app/routes/defer.tsx": js` - export let loader = () => ({ data: 'whatever' }); + export let loader = () => ({ data: Promise.resolve('whatever') }); `, "app/routes/data[.]json.tsx": js` export let loader = () => Response.json({ hello: "world" }); @@ -101,6 +101,11 @@ test.describe("loader in an app", async () => { return { hello: 'world' }; } `, + "app/routes/return-string.tsx": js` + export let loader = () => { + return 'hello world'; + } + `, "app/routes/throw-object.tsx": js` export let loader = () => { throw { but: 'why' }; @@ -207,9 +212,25 @@ test.describe("loader in an app", async () => { expect(await res.text()).toEqual("Partial"); }); - // TODO: This test should work once we bring over the changes from - // https://github.com/remix-run/remix/pull/9349 to the v7 branch - test.skip("should handle objects returned from resource routes", async ({ + test("should convert strings returned from resource routes to text responses", async ({ + page, + }) => { + let app = new PlaywrightFixture(appFixture, page); + let res = await app.goto("/return-string"); + expect(res.status()).toBe(200); + expect(await res.text()).toEqual("hello world"); + }); + + test("should convert non-strings returned from resource routes to JSON responses", async ({ + page, + }) => { + let app = new PlaywrightFixture(appFixture, page); + let res = await app.goto("/return-object"); + expect(res.status()).toBe(200); + expect(await res.json()).toEqual({ hello: "world" }); + }); + + test("should handle objects returned from resource routes", async ({ page, }) => { let app = new PlaywrightFixture(appFixture, page); @@ -256,10 +277,8 @@ test.describe("loader in an app", async () => { }) => { let app = new PlaywrightFixture(appFixture, page); let res = await app.goto("/defer"); - expect(res.status()).toBe(500); - expect(await res.text()).toMatch( - "Error: Expected a Response to be returned from resource route handler" - ); + expect(res.status()).toBe(200); + expect(await res.json()).toEqual({ data: {} }); }); }); diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index f7c090eb50..4810ea371f 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -504,12 +504,15 @@ async function handleResourceRequest( requestContext: loadContext, }); - invariant( - isResponse(response), - "Expected a Response to be returned from resource route handler" - ); + if (isResponse(response)) { + return response; + } - return response; + if (typeof response === "string") { + return new Response(response); + } + + return Response.json(response); } catch (error: unknown) { if (isResponse(error)) { // Note: Not functionally required but ensures that our response headers