Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/new-hornets-run.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": minor
---

Add a new `dataRedirect` utility for performing redirects on `.data` requests outside of the React Router handler
3 changes: 3 additions & 0 deletions integration/helpers/vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export const EXPRESS_SERVER = (args: {
port: number;
base?: string;
loadContext?: Record<string, unknown>;
customLogic: string;
}) =>
String.raw`
import { createRequestHandler } from "@react-router/express";
Expand All @@ -130,6 +131,8 @@ export const EXPRESS_SERVER = (args: {
}
app.use(express.static("build/client", { maxAge: "1h" }));

${args?.customLogic || ""}

app.all(
"*",
createRequestHandler({
Expand Down
70 changes: 69 additions & 1 deletion integration/single-fetch-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ import {
js,
} from "./helpers/create-fixture.js";
import { PlaywrightFixture } from "./helpers/playwright-fixture.js";
import { reactRouterConfig } from "./helpers/vite.js";
import {
EXPRESS_SERVER,
createProject,
customDev,
reactRouterConfig,
viteConfig,
} from "./helpers/vite.js";
import getPort from "get-port";

const ISO_DATE = "2024-03-12T12:00:00.000Z";

Expand Down Expand Up @@ -1464,6 +1471,67 @@ test.describe("single-fetch", () => {
expect(await app.getHtml("#target")).toContain("Target");
});

test("processes redirects returned outside of react router", async ({
page,
}) => {
let port = await getPort();
let cwd = await createProject({
"vite.config.js": await viteConfig.basic({ port }),
"server.mjs": EXPRESS_SERVER({
port,
customLogic: js`
app.use(async (req, res, next) => {
if (req.url === "/page.data") {
let { dataRedirect } = await import("react-router");
let response = dataRedirect("/target");
res.statusMessage = response.statusText;
res.status(response.status);
for (let [key, value] of response.headers.entries()) {
res.append(key, value);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't export our internal logic for converting a fetch Response to an express res so for now it's assumed that the application has to do that adapter logic on their own. I think this is ok since status/headers are all that matter? There isn't a body on these responses

res.end();
} else {
next();
}
});
`,
}),
"app/routes/_index.tsx": js`
import { Link } from "react-router";
export default function Component() {
return <Link to="/page">Go to /page</Link>
}
`,
"app/routes/page.tsx": js`
export function loader() {
return null
}
export default function Component() {
return <p>Should not see me</p>
}
`,
"app/routes/target.tsx": js`
export default function Component() {
return <h1 id="target">Target</h1>
}
`,
});
let stop = await customDev({ cwd, port });

try {
await page.goto(`http://localhost:${port}/`, {
waitUntil: "networkidle",
});
let link = page.locator('a[href="/page"]');
await expect(link).toHaveText("Go to /page");
await link.click();
await page.waitForSelector("#target");
await expect(page.locator("#target")).toHaveText("Target");
} finally {
stop();
}
});

test("processes thrown loader errors", async ({ page }) => {
let fixture = await createFixture({
files: {
Expand Down
1 change: 1 addition & 0 deletions packages/react-router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export {
} from "./lib/router/router";
export {
data,
dataRedirect,
generatePath,
isRouteErrorResponse,
matchPath,
Expand Down
26 changes: 23 additions & 3 deletions packages/react-router/lib/dom/ssr/single-fetch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { escapeHtml } from "./markup";
import type { RouteModule, RouteModules } from "./routeModules";
import invariant from "./invariant";
import type { EntryRoute } from "./routes";
import { SINGLE_FETCH_REDIRECT_STATUS } from "../../server-runtime/single-fetch";

export const SingleFetchRedirectSymbol = Symbol("SingleFetchRedirect");

Expand Down Expand Up @@ -550,6 +551,25 @@ async function fetchAndDecode(
throw new ErrorResponseImpl(404, "Not Found", true);
}

// Handle non-RR redirects (i.e., from express middleware via `dataRedirect`)
if (res.status === 204) {
let data: SingleFetchRedirectResult = {
redirect: res.headers.get("X-Remix-Redirect")!,
status: Number(res.headers.get("X-Remix-Status") || "302"),
revalidate: res.headers.get("X-Remix-Revalidate") === "true",
reload: res.headers.get("X-Remix-Reload-Document") === "true",
replace: res.headers.get("X-Remix-Replace") === "true",
};
if (!init.method || init.method === "GET") {
return {
status: SINGLE_FETCH_REDIRECT_STATUS,
data: { [SingleFetchRedirectSymbol]: data },
};
} else {
return { status: SINGLE_FETCH_REDIRECT_STATUS, data };
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we get back a 204 redirect we convert it to a single fetch redirect here for downstream processing


// some status codes are not permitted to have bodies, so we want to just
// treat those as "no data" instead of throwing an exception.
// 304 is not included here because the browser should fill those responses
Expand Down Expand Up @@ -657,13 +677,13 @@ function unwrapSingleFetchResult(result: SingleFetchResult, routeId: string) {
} else if ("redirect" in result) {
let headers: Record<string, string> = {};
if (result.revalidate) {
headers["X-Remix-Revalidate"] = "yes";
headers["X-Remix-Revalidate"] = "true";
}
if (result.reload) {
headers["X-Remix-Reload-Document"] = "yes";
headers["X-Remix-Reload-Document"] = "true";
}
if (result.replace) {
headers["X-Remix-Replace"] = "yes";
headers["X-Remix-Replace"] = "true";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align these with values used elsewhere - value doesn't really matter since we use .has(header)

}
throw redirect(result.redirect, { status: result.status, headers });
} else if ("data" in result) {
Expand Down
31 changes: 31 additions & 0 deletions packages/react-router/lib/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1605,6 +1605,37 @@ export const replace: RedirectFunction = (url, init) => {
return response;
};

/**
* A "soft" redirect response that can be returned from outside of React Router
* in response to a `.data` request. Options are available to set the status
* and toggle the equivalent behaviors of the traditional redirect utilities
* ()`redirect`/`replace`/`redirectDocument`)
*
* @category Utils
*/
export function dataRedirect(
url: string,
{
status,
replace,
revalidate,
reload,
}: {
status?: number;
replace?: boolean;
revalidate?: boolean;
reload?: boolean;
} = {}
) {
let headers = new Headers();
headers.set("X-Remix-Redirect", url);
if (status) headers.set("X-Remix-Status", String(status));
if (replace) headers.set("X-Remix-Replace", "true");
if (revalidate) headers.set("X-Remix-Revalidate", "true");
if (reload) headers.set("X-Remix-Reload-Document", "true");
return new Response(null, { status: 204, headers });
}

export type ErrorResponse = {
status: number;
statusText: string;
Expand Down
Loading