diff --git a/.changeset/quiet-cows-build.md b/.changeset/quiet-cows-build.md new file mode 100644 index 0000000000..67c67a9776 --- /dev/null +++ b/.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 diff --git a/integration/middleware-test.ts b/integration/middleware-test.ts index 0dd1436f51..34b31a9db4 100644 --- a/integration/middleware-test.ts +++ b/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

Redirect

+ } + `, + "app/routes/target.tsx": js` + export default function Component() { + return

Target

+ } + `, + }, + }); + + 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, }) => { diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index 3a19718e47..728467525e 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/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, ); } } diff --git a/packages/react-router/lib/server-runtime/single-fetch.ts b/packages/react-router/lib/server-runtime/single-fetch.ts index 741e19ea26..7236a061e7 100644 --- a/packages/react-router/lib/server-runtime/single-fetch.ts +++ b/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>, ) { - 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,