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/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); diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index 8075d660e76..67d2189e205 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("/"); @@ -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("/"); @@ -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("/"); @@ -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("/"); @@ -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("/"); @@ -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("/"); @@ -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("/"); @@ -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("/"); @@ -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("/"); @@ -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("/"); @@ -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: { diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 4c5d7568261..536a60783a0 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -35,6 +35,7 @@ import { createServerHandoffString } from "./serverHandoff"; import { getDevServerHooks } from "./dev"; import type { SingleFetchResult, SingleFetchResults } from "./single-fetch"; import { + convertResponseStubToErrorResponse, encodeViaTurboStream, getResponseStubs, getSingleFetchDataStrategy, @@ -46,7 +47,7 @@ import { singleFetchLoaders, SingleFetchRedirectSymbol, ResponseStubOperationsSymbol, - convertResponseStubToErrorResponse, + SINGLE_FETCH_REDIRECT_STATUS, } from "./single-fetch"; import { resourceRouteJsonWarning } from "./deprecations"; @@ -219,7 +220,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 4ca48bb4bd4..4c31135c573 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, }; } @@ -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, }; } @@ -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, }; }