Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single Fetch: fix issues returning/throwing response stubs from resource routes #9488

Merged
merged 2 commits into from
May 28, 2024
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
6 changes: 6 additions & 0 deletions .changeset/smart-mirrors-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@remix-run/server-runtime": patch
---

- Fix error when returning null from a resource route in single fetch
- Fix issues with returning or throwing a response stub from a resource route in single fetch
111 changes: 111 additions & 0 deletions integration/single-fetch-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1539,6 +1539,33 @@ test.describe("single-fetch", () => {
console.warn = oldConsoleWarn;
});

test("allows resource routes to return null using a response stub", async () => {
let fixture = await createFixture(
{
config: {
future: {
unstable_singleFetch: true,
},
},
files: {
...files,
"app/routes/resource.tsx": js`
export function loader({ response }) {
response.status = 201;
response.headers.set('X-Created-Id', '1');
return null;
}
`,
},
},
ServerMode.Development
);
let res = await fixture.requestResource("/resource");
expect(res.status).toBe(201);
expect(res.headers.get("X-Created-Id")).toBe("1");
expect(await res.text()).toBe("");
});

test("processes response stub onto resource routes returning raw data", async () => {
let fixture = await createFixture(
{
Expand Down Expand Up @@ -1618,6 +1645,90 @@ test.describe("single-fetch", () => {
});
});

test("processes returned response stub redirects", async () => {
let fixture = await createFixture(
{
config: {
future: {
unstable_singleFetch: true,
},
},
files: {
...files,
"app/routes/resource.tsx": js`
import { json } from '@remix-run/node';

export function loader({ response }) {
response.status = 301;
response.headers.set('Location', '/whatever')
return response;
}
`,
},
},
ServerMode.Development
);
let res = await fixture.requestResource("/resource");
expect(res.status).toBe(301);
expect(res.headers.get("Location")).toBe("/whatever");
});

test("processes thrown response stub redirects", async () => {
let fixture = await createFixture(
{
config: {
future: {
unstable_singleFetch: true,
},
},
files: {
...files,
"app/routes/resource.tsx": js`
import { json } from '@remix-run/node';

export function loader({ response }) {
response.status = 301;
response.headers.set('Location', '/whatever')
throw response;
}
`,
},
},
ServerMode.Development
);
let res = await fixture.requestResource("/resource");
expect(res.status).toBe(301);
expect(res.headers.get("Location")).toBe("/whatever");
});

test("processes response stub redirects when null is returned", async () => {
let fixture = await createFixture(
{
config: {
future: {
unstable_singleFetch: true,
},
},
files: {
...files,
"app/routes/resource.tsx": js`
import { json } from '@remix-run/node';

export function loader({ response }) {
response.status = 301;
response.headers.set('Location', '/whatever')
return null;
}
`,
},
},
ServerMode.Development
);
let res = await fixture.requestResource("/resource");
expect(res.status).toBe(301);
expect(res.headers.get("Location")).toBe("/whatever");
});

test("allows fetcher to hit resource route and return via turbo stream", async ({
page,
}) => {
Expand Down
16 changes: 15 additions & 1 deletion packages/remix-server-runtime/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ async function handleResourceRequest(
: null),
});

if (typeof response === "object") {
if (typeof response === "object" && response !== null) {
invariant(
!(DEFERRED_SYMBOL in response),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoids cannot search for property in null error

`You cannot return a \`defer()\` response from a Resource Route. Did you ` +
Expand All @@ -609,6 +609,13 @@ async function handleResourceRequest(
// @ts-expect-error
response.headers[op](...args);
}
} else if (isResponseStub(response) || response == null) {
// If the stub or null was returned, then there is no body so we just
// proxy along the status/headers to a Response
response = new Response(null, {
status: stub.status,
headers: stub.headers,
});
} else {
console.warn(
resourceRouteJsonWarning(
Expand Down Expand Up @@ -639,6 +646,13 @@ async function handleResourceRequest(
return error;
}

if (isResponseStub(error)) {
return new Response(null, {
status: error.status,
headers: error.headers,
});
}

if (isRouteErrorResponse(error)) {
if (error) {
handleError(error);
Expand Down