Skip to content

Commit

Permalink
Loosen restrictions on resource route (#12848)
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 authored Jan 23, 2025
1 parent e23d6cb commit 80cf6ff
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
8 changes: 8 additions & 0 deletions .changeset/kind-hats-greet.md
Original file line number Diff line number Diff line change
@@ -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`
35 changes: 27 additions & 8 deletions integration/resource-routes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ test.describe("loader in an app", async () => {
export default () => <div data-testid="redirect-destination">You made it!</div>
`,
"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" });
Expand Down Expand Up @@ -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' };
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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: {} });
});
});

Expand Down
13 changes: 8 additions & 5 deletions packages/react-router/lib/server-runtime/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 80cf6ff

Please sign in to comment.