Delay turbo-stream serialization of .data redirects (#14205)

Author: brophdawg11

.changeset/quiet-cows-build.md

@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+[UNSTABLE] Delay serialization of `.data` redirects to 202 responses until after middleware chain

integration/middleware-test.ts

@@ -3,7 +3,11 @@ import type {
   Response as PlaywrightResponse,
 } from "@playwright/test";
 import { test, expect } from "@playwright/test";
-import { UNSAFE_ErrorResponseImpl, UNSAFE_ServerMode } from "react-router";
+import {
+  UNSAFE_ErrorResponseImpl,
+  UNSAFE_ServerMode,
+  UNSAFE_SingleFetchRedirectSymbol,
+} from "react-router";
 
 import {
   createAppFixture,
@@ -1876,8 +1880,12 @@ test.describe("Middleware", () => {
         },
       });
 
-      let appFixture = await createAppFixture(fixture);
+      let res = await fixture.requestDocument("/redirect");
+      expect(res.status).toBe(302);
+      expect(res.headers.get("location")).toBe("/target");
+      expect(res.body).toBeNull();
 
+      let appFixture = await createAppFixture(fixture);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/");
       await page.waitForSelector('a:has-text("Link")');
@@ -1933,8 +1941,12 @@ test.describe("Middleware", () => {
         },
       });
 
-      let appFixture = await createAppFixture(fixture);
+      let res = await fixture.requestDocument("/redirect");
+      expect(res.status).toBe(302);
+      expect(res.headers.get("location")).toBe("/target");
+      expect(res.body).toBeNull();
 
+      let appFixture = await createAppFixture(fixture);
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/");
       await page.waitForSelector('a:has-text("Link")');
@@ -1945,6 +1957,66 @@ test.describe("Middleware", () => {
       appFixture.close();
     });
 
+    test("doesn't serialize single fetch redirects until after the middleware chain", async ({
+      page,
+    }) => {
+      let fixture = await createFixture({
+        files: {
+          "react-router.config.ts": reactRouterConfig({
+            middleware: true,
+          }),
+          "vite.config.ts": js`
+            import { defineConfig } from "vite";
+            import { reactRouter } from "@react-router/dev/vite";
+
+            export default defineConfig({
+              build: { manifest: true },
+              plugins: [reactRouter()],
+            });
+          `,
+          "app/routes/redirect.tsx": js`
+            import { Link, redirect } from 'react-router'
+            export const unstable_middleware = [
+              async ({ request, context }, next) => {
+                let res = await next();
+                // Should still be a normal redirect here, not yet encoded into
+                // a single fetch redirect
+                res.headers.set("X-Status", res.status);
+                res.headers.set("X-Location", res.headers.get('Location'));
+                return res;
+              }
+            ]
+            export function loader() {
+              throw redirect('/target');
+            }
+            export default function Component() {
+              return <h1>Redirect</h1>
+            }
+          `,
+          "app/routes/target.tsx": js`
+            export default function Component() {
+              return <h1>Target</h1>
+            }
+          `,
+        },
+      });
+
+      let res = await fixture.requestSingleFetchData("/redirect.data");
+      expect(res.status).toBe(202);
+      expect(res.headers.get("location")).toBe(null);
+      expect(res.headers.get("x-status")).toBe("302");
+      expect(res.headers.get("x-location")).toBe("/target");
+      expect(res.data).toEqual({
+        [UNSAFE_SingleFetchRedirectSymbol]: {
+          redirect: "/target",
+          reload: false,
+          replace: false,
+          revalidate: false,
+          status: 302,
+        },
+      });
+    });
+
     test("handles errors thrown on the way down (document)", async ({
       page,
     }) => {

packages/react-router/lib/server-runtime/server.ts

@@ -27,21 +27,13 @@ import { createServerHandoffString } from "./serverHandoff";
 import { getBuildTimeHeader, getDevServerHooks } from "./dev";
 import {
   encodeViaTurboStream,
-  getSingleFetchRedirect,
   singleFetchAction,
   singleFetchLoaders,
   SERVER_NO_BODY_STATUS_CODES,
+  generateSingleFetchRedirectResponse,
 } from "./single-fetch";
 import { getDocumentHeaders } from "./headers";
 import type { EntryRoute } from "../dom/ssr/routes";
-import type {
-  SingleFetchResult,
-  SingleFetchResults,
-} from "../dom/ssr/single-fetch";
-import {
-  SINGLE_FETCH_REDIRECT_STATUS,
-  SingleFetchRedirectSymbol,
-} from "../dom/ssr/single-fetch";
 import type { MiddlewareEnabled } from "../types/future";
 import { getManifestPath } from "../dom/ssr/fog-of-war";
 
@@ -269,6 +261,15 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
         handleError,
       );
 
+      if (isRedirectResponse(response)) {
+        response = generateSingleFetchRedirectResponse(
+          response,
+          request,
+          _build,
+          serverMode,
+        );
+      }
+
       if (_build.entry.module.handleDataRequest) {
         response = await _build.entry.module.handleDataRequest(response, {
           context: loadContext,
@@ -277,32 +278,11 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
         });
 
         if (isRedirectResponse(response)) {
-          let result: SingleFetchResult | SingleFetchResults =
-            getSingleFetchRedirect(
-              response.status,
-              response.headers,
-              _build.basename,
-            );
-
-          if (request.method === "GET") {
-            result = {
-              [SingleFetchRedirectSymbol]: result,
-            };
-          }
-          let headers = new Headers(response.headers);
-          headers.set("Content-Type", "text/x-script");
-
-          return new Response(
-            encodeViaTurboStream(
-              result,
-              request.signal,
-              _build.entry.module.streamTimeout,
-              serverMode,
-            ),
-            {
-              status: SINGLE_FETCH_REDIRECT_STATUS,
-              headers,
-            },
+          response = generateSingleFetchRedirectResponse(
+            response,
+            request,
+            _build,
+            serverMode,
           );
         }
       }

packages/react-router/lib/server-runtime/single-fetch.ts

@@ -1,10 +1,6 @@
 import { encode } from "../../vendor/turbo-stream-v2/turbo-stream";
 import type { StaticHandler, StaticHandlerContext } from "../router/router";
-import {
-  isRedirectResponse,
-  isRedirectStatusCode,
-  isResponse,
-} from "../router/router";
+import { isRedirectStatusCode, isResponse } from "../router/router";
 import type { unstable_RouterContextProvider } from "../router/utils";
 import {
   isRouteErrorResponse,
@@ -78,25 +74,7 @@ export async function singleFetchAction(
   function handleQueryResult(
     result: Awaited<ReturnType<StaticHandler["query"]>>,
   ) {
-    if (!isResponse(result)) {
-      result = staticContextToResponse(result);
-    }
-
-    // Unlike `handleDataRequest`, when singleFetch is enabled, query does
-    // let non-Response return values through
-    if (isRedirectResponse(result)) {
-      return generateSingleFetchResponse(request, build, serverMode, {
-        result: getSingleFetchRedirect(
-          result.status,
-          result.headers,
-          build.basename,
-        ),
-        headers: result.headers,
-        status: SINGLE_FETCH_REDIRECT_STATUS,
-      });
-    }
-
-    return result;
+    return isResponse(result) ? result : staticContextToResponse(result);
   }
 
   function handleQueryError(error: unknown) {
@@ -113,15 +91,7 @@ export async function singleFetchAction(
     let headers = getDocumentHeaders(context, build);
 
     if (isRedirectStatusCode(context.statusCode) && headers.has("Location")) {
-      return generateSingleFetchResponse(request, build, serverMode, {
-        result: getSingleFetchRedirect(
-          context.statusCode,
-          headers,
-          build.basename,
-        ),
-        headers,
-        status: SINGLE_FETCH_REDIRECT_STATUS,
-      });
+      return new Response(null, { status: context.statusCode, headers });
     }
 
     // Sanitize errors outside of development environments
@@ -194,24 +164,7 @@ export async function singleFetchLoaders(
   // Handle the query() result - either inside stream() with middleware enabled
   // or after query() without
   function handleQueryResult(result: StaticHandlerContext | Response) {
-    let response = isResponse(result)
-      ? result
-      : staticContextToResponse(result);
-    if (isRedirectResponse(response)) {
-      return generateSingleFetchResponse(request, build, serverMode, {
-        result: {
-          [SingleFetchRedirectSymbol]: getSingleFetchRedirect(
-            response.status,
-            response.headers,
-            build.basename,
-          ),
-        },
-        headers: response.headers,
-        status: SINGLE_FETCH_REDIRECT_STATUS,
-      });
-    }
-
-    return response;
+    return isResponse(result) ? result : staticContextToResponse(result);
   }
 
   // Handle any thrown errors from query() result - either inside stream() with
@@ -230,17 +183,7 @@ export async function singleFetchLoaders(
     let headers = getDocumentHeaders(context, build);
 
     if (isRedirectStatusCode(context.statusCode) && headers.has("Location")) {
-      return generateSingleFetchResponse(request, build, serverMode, {
-        result: {
-          [SingleFetchRedirectSymbol]: getSingleFetchRedirect(
-            context.statusCode,
-            headers,
-            build.basename,
-          ),
-        },
-        headers,
-        status: SINGLE_FETCH_REDIRECT_STATUS,
-      });
+      return new Response(null, { status: context.statusCode, headers });
     }
 
     // Sanitize errors outside of development environments
@@ -334,6 +277,32 @@ function generateSingleFetchResponse(
   );
 }
 
+export function generateSingleFetchRedirectResponse(
+  redirectResponse: Response,
+  request: Request,
+  build: ServerBuild,
+  serverMode: ServerMode,
+) {
+  let redirect = getSingleFetchRedirect(
+    redirectResponse.status,
+    redirectResponse.headers,
+    build.basename,
+  );
+
+  let headers = new Headers(redirectResponse.headers);
+  headers.delete("Location");
+  headers.set("Content-Type", "text/x-script");
+
+  return generateSingleFetchResponse(request, build, serverMode, {
+    result:
+      request.method === "GET"
+        ? { [SingleFetchRedirectSymbol]: redirect }
+        : redirect,
+    headers,
+    status: SINGLE_FETCH_REDIRECT_STATUS,
+  });
+}
+
 export function getSingleFetchRedirect(
   status: number,
   headers: Headers,