From 78b9f00a9fe6029b22636eb50ca709c181835e79 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 5 Jun 2024 11:14:41 -0400 Subject: [PATCH 1/2] Update single fetch redirects to use a 202 status code on data requests --- .changeset/neat-ghosts-thank.md | 5 + integration/single-fetch-test.ts | 124 +++++++++++++++++- packages/remix-server-runtime/server.ts | 3 +- packages/remix-server-runtime/single-fetch.ts | 15 ++- 4 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 .changeset/neat-ghosts-thank.md diff --git a/.changeset/neat-ghosts-thank.md b/.changeset/neat-ghosts-thank.md new file mode 100644 index 00000000000..82d6992aa0d --- /dev/null +++ b/.changeset/neat-ghosts-thank.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Change single fetch redirects to use a 202 status to avoid automatic caching diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index a3aa4f0d039..da66b936119 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -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"; @@ -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("/"); @@ -1045,6 +1057,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("/"); @@ -1086,6 +1109,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("/"); @@ -1124,6 +1158,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("/"); @@ -1175,6 +1220,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("/"); @@ -1224,6 +1281,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("/"); @@ -1271,6 +1340,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("/"); @@ -1316,6 +1397,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("/"); @@ -1404,6 +1497,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("/"); @@ -1492,6 +1597,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("/"); @@ -1645,7 +1763,7 @@ test.describe("single-fetch", () => { }); }); - test("processes returned response stub redirects", async () => { + test("processes returned response stub redirects from resource route", async () => { let fixture = await createFixture( { config: { @@ -1673,7 +1791,7 @@ test.describe("single-fetch", () => { expect(res.headers.get("Location")).toBe("/whatever"); }); - test("processes thrown response stub redirects", async () => { + test("processes thrown response stub redirects from resource route", async () => { let fixture = await createFixture( { config: { @@ -1701,7 +1819,7 @@ test.describe("single-fetch", () => { expect(res.headers.get("Location")).toBe("/whatever"); }); - test("processes response stub redirects when null is returned", async () => { + test("processes response stub redirects when null is returned from resource route", async () => { let fixture = await createFixture( { config: { diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 566c515ecfc..17cd0663d38 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -46,6 +46,7 @@ import { singleFetchLoaders, SingleFetchRedirectSymbol, ResponseStubOperationsSymbol, + SINGLE_FETCH_REDIRECT_STATUS, } from "./single-fetch"; import { resourceRouteJsonWarning } from "./deprecations"; @@ -218,7 +219,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( serverMode ), { - status: 200, + status: SINGLE_FETCH_REDIRECT_STATUS, headers, } ); diff --git a/packages/remix-server-runtime/single-fetch.ts b/packages/remix-server-runtime/single-fetch.ts index 7571c8478df..b06c30bbe98 100644 --- a/packages/remix-server-runtime/single-fetch.ts +++ b/packages/remix-server-runtime/single-fetch.ts @@ -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, { @@ -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, }; } @@ -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, }; } @@ -246,7 +253,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, }; } @@ -263,7 +270,7 @@ export async function singleFetchLoaders( ), }, headers, - status: 200, // Don't want the `fetch` call to follow the redirect + status: SINGLE_FETCH_REDIRECT_STATUS, }; } From e5d805e98b8ee44c10d625a865d54a3f02749674 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 6 Jun 2024 10:19:23 -0400 Subject: [PATCH 2/2] Update action test --- integration/action-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/action-test.ts b/integration/action-test.ts index b6de9ef020d..3c803eea732 100644 --- a/integration/action-test.ts +++ b/integration/action-test.ts @@ -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);