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

Update single fetch redirects to use a 202 status code on data requests #9564

Merged
merged 3 commits into from
Jun 13, 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
5 changes: 5 additions & 0 deletions .changeset/neat-ghosts-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": patch
---

Change single fetch redirects to use a 202 status to avoid automatic caching
2 changes: 1 addition & 1 deletion integration/action-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ test.describe("single fetch", () => {
await page.waitForSelector(`#${REDIRECT_TARGET}`);

expect(responses.length).toBe(1);
expect(responses[0].status()).toBe(200);
expect(responses[0].status()).toBe(202);

expect(new URL(page.url()).pathname).toBe(`/${REDIRECT_TARGET}`);
expect(await app.getHtml()).toMatch(PAGE_TEXT);
Expand Down
176 changes: 175 additions & 1 deletion integration/single-fetch-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from "./helpers/create-fixture.js";
import { PlaywrightFixture } from "./helpers/playwright-fixture.js";
import { ServerMode } from "../build/node_modules/@remix-run/server-runtime/dist/mode.js";
import { SingleFetchRedirectSymbol } from "../build/node_modules/@remix-run/server-runtime/dist/single-fetch.js";

const ISO_DATE = "2024-03-12T12:00:00.000Z";

Expand Down Expand Up @@ -1002,6 +1003,17 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data");
expect(data).toEqual({
[SingleFetchRedirectSymbol]: {
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
},
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1130,6 +1142,17 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data");
expect(data).toEqual({
[SingleFetchRedirectSymbol]: {
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
},
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1251,6 +1274,17 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data");
expect(data).toEqual({
[SingleFetchRedirectSymbol]: {
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
},
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1289,6 +1323,17 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data");
expect(data).toEqual({
[SingleFetchRedirectSymbol]: {
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
},
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1340,6 +1385,18 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data", {
method: "post",
body: null,
});
expect(data).toEqual({
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture, ServerMode.Development);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1486,6 +1543,18 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data", {
method: "post",
body: null,
});
expect(data).toEqual({
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture, ServerMode.Development);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1625,6 +1694,18 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data", {
method: "post",
body: null,
});
expect(data).toEqual({
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture, ServerMode.Development);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1670,6 +1751,18 @@ test.describe("single-fetch", () => {
expect(res.headers.get("Location")).toBe("/target");
expect(await res.text()).toBe("");

let { status, data } = await fixture.requestSingleFetchData("/data.data", {
method: "post",
body: null,
});
expect(data).toEqual({
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture, ServerMode.Development);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1758,6 +1851,18 @@ test.describe("single-fetch", () => {
`,
},
});

let { status, data } = await fixture.requestSingleFetchData("/data.data");
expect(data).toEqual({
[SingleFetchRedirectSymbol]: {
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
},
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -1846,6 +1951,19 @@ test.describe("single-fetch", () => {
`,
},
});

let { status, data } = await fixture.requestSingleFetchData("/data.data", {
method: "post",
body: null,
});
expect(data).toEqual({
status: 302,
redirect: "/target",
reload: false,
revalidate: false,
});
expect(status).toBe(202);

let appFixture = await createAppFixture(fixture);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down Expand Up @@ -2094,7 +2212,63 @@ test.describe("single-fetch", () => {
});
});

test("processes response stub redirects when null is returned", async () => {
test("processes returned response stub redirects from resource route", 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 from resource route", 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 from resource route", async () => {
let fixture = await createFixture(
{
config: {
Expand Down
5 changes: 3 additions & 2 deletions packages/remix-server-runtime/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { createServerHandoffString } from "./serverHandoff";
import { getDevServerHooks } from "./dev";
import type { SingleFetchResult, SingleFetchResults } from "./single-fetch";
import {
convertResponseStubToErrorResponse,
encodeViaTurboStream,
getResponseStubs,
getSingleFetchDataStrategy,
Expand All @@ -46,7 +47,7 @@ import {
singleFetchLoaders,
SingleFetchRedirectSymbol,
ResponseStubOperationsSymbol,
convertResponseStubToErrorResponse,
SINGLE_FETCH_REDIRECT_STATUS,
} from "./single-fetch";
import { resourceRouteJsonWarning } from "./deprecations";

Expand Down Expand Up @@ -219,7 +220,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
serverMode
),
{
status: 200,
status: SINGLE_FETCH_REDIRECT_STATUS,
headers,
}
);
Expand Down
15 changes: 11 additions & 4 deletions packages/remix-server-runtime/single-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ export type DataStrategyCtx = {
response: ResponseStub;
};

// We can't use a 3xx status or else the `fetch()` would follow the redirect.
// We need to communicate the redirect back as data so we can act on it in the
// client side router. We use a 202 to avoid any automatic caching we might
// get from a 200 since a "temporary" redirect should not be cached. This lets
// the user control cache behavior via Cache-Control
export const SINGLE_FETCH_REDIRECT_STATUS = 202;

export function getSingleFetchDataStrategy(
responseStubs: ReturnType<typeof getResponseStubs>,
{
Expand Down Expand Up @@ -159,7 +166,7 @@ export async function singleFetchAction(
return {
result: getSingleFetchRedirect(result.status, result.headers),
headers: result.headers,
status: 200,
status: SINGLE_FETCH_REDIRECT_STATUS,
};
}

Expand All @@ -174,7 +181,7 @@ export async function singleFetchAction(
return {
result: getSingleFetchRedirect(statusCode, headers),
headers,
status: 200, // Don't want the `fetch` call to follow the redirect
status: SINGLE_FETCH_REDIRECT_STATUS,
};
}

Expand Down Expand Up @@ -250,7 +257,7 @@ export async function singleFetchLoaders(
),
},
headers: result.headers,
status: 200, // Don't want the `fetch` call to follow the redirect
status: SINGLE_FETCH_REDIRECT_STATUS,
};
}

Expand All @@ -267,7 +274,7 @@ export async function singleFetchLoaders(
),
},
headers,
status: 200, // Don't want the `fetch` call to follow the redirect
status: SINGLE_FETCH_REDIRECT_STATUS,
};
}

Expand Down