Skip to content

Add a new dataRedirect utility for external redirects on .data requests #13364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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);
}
Comment on lines +1487 to +1491
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 && res.headers.has("X-Remix-Redirect")) {
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