Update single fetch redirects to use a 202 status code on data reques…

…ts (#9564)
remix-run · Jun 13, 2024 · d0b6860 · d0b6860
1 parent f1c8fa7
commit d0b6860
Show file tree

Hide file tree

Showing 5 changed files with 195 additions and 8 deletions.
diff --git a/.changeset/neat-ghosts-thank.md 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
@@ -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
@@ -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
@@ -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
@@ -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>,
   {
@@ -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,
       };
     }