Skip to content

Fix react-router-serve for prerendered files and avoid dup loader invocation #12071

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

Merged
merged 7 commits into from
Oct 7, 2024
Merged
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
8 changes: 8 additions & 0 deletions .changeset/rare-plums-chew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@react-router/serve": patch
"@react-router/dev": patch
"react-router": patch
---

- Fix `react-router-serve` handling of prerendered HTML files by removing the `redirect: false` option so it now falls back on the default `redirect: true` behavior of redirecting from `/folder` -> `/folder/` which will then pick up `/folder/index.html` from disk. See https://expressjs.com/en/resources/middleware/serve-static.html
- Proxy prerendered loader data into prerender pass for HTML files to avoid double-invocations of the loader at build time
14 changes: 1 addition & 13 deletions docs/misc/pre-rendering.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,7 @@ app.use(
);

// Serve static HTML and .data requests without Cache-Control
app.use(
"/",
express.static("build/client", {
// Don't redirect directory index.html requests to include a trailing slash
redirect: false,
setHeaders: function (res, path) {
// Add the proper Content-Type for turbo-stream data responses
if (path.endsWith(".data")) {
res.set("Content-Type", "text/x-turbo");
}
},
})
);
app.use("/", express.static("build/client"));

// Serve remaining unhandled requests via your React Router handler
app.all(
Expand Down
57 changes: 57 additions & 0 deletions integration/vite-prerender-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,63 @@ test.describe("Prerendering", () => {
expect(await app.getHtml()).toContain("<span>NOT-PRERENDERED-false</span>");
});

test("Does not encounter header limits on large prerendered data", async ({
page,
}) => {
fixture = await createFixture({
// Even thogh we are prerendering, we want a running server so we can
// hit the pre-rendered HTML file and a non-prerendered route
prerender: false,
files: {
...files,
"vite.config.ts": js`
import { defineConfig } from "vite";
import { reactRouter } from "@react-router/dev/vite";

export default defineConfig({
build: { manifest: true },
plugins: [
reactRouter({
prerender: ["/", "/about"],
})
],
});
`,
"app/routes/about.tsx": js`
import { useLoaderData } from 'react-router';
export function loader({ request }) {
return {
prerendered: request.headers.has('X-React-Router-Prerender') ? 'yes' : 'no',
// 24999 characters
data: new Array(5000).fill('test').join('-'),
};
}

export default function Comp() {
let data = useLoaderData();
return (
<>
<h1 data-title>Large loader</h1>
<p data-prerendered>{data.prerendered}</p>
<p data-length>{data.data.length}</p>
</>
);
}
`,
},
});
appFixture = await createAppFixture(fixture);

let app = new PlaywrightFixture(appFixture, page);
await app.goto("/about");
await page.waitForSelector("[data-mounted]");
expect(await app.getHtml("[data-title]")).toContain("Large loader");
expect(await app.getHtml("[data-prerendered]")).toContain("yes");
expect(await app.getHtml("[data-length]")).toBe(
'<p data-length="true">24999</p>'
);
});

test("Renders down to the proper HydrateFallback", async ({ page }) => {
fixture = await createFixture({
prerender: true,
Expand Down
20 changes: 11 additions & 9 deletions packages/react-router-dev/vite/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1831,23 +1831,22 @@ async function handlePrerender(
} else {
routesToPrerender = reactRouterConfig.prerender || ["/"];
}
let requestInit = {
headers: {
// Header that can be used in the loader to know if you're running at
// build time or runtime
"X-React-Router-Prerender": "yes",
},
let headers = {
// Header that can be used in the loader to know if you're running at
// build time or runtime
"X-React-Router-Prerender": "yes",
};
for (let path of routesToPrerender) {
let hasLoaders = matchRoutes(routes, path)?.some((m) => m.route.loader);
let data: string | undefined;
if (hasLoaders) {
await prerenderData(
data = await prerenderData(
handler,
path,
clientBuildDirectory,
reactRouterConfig,
viteConfig,
requestInit
{ headers }
);
}
await prerenderRoute(
Expand All @@ -1856,7 +1855,9 @@ async function handlePrerender(
clientBuildDirectory,
reactRouterConfig,
viteConfig,
requestInit
data
? { headers: { ...headers, "X-React-Router-Prerender-Data": data } }
: { headers }
);
}

Expand Down Expand Up @@ -1934,6 +1935,7 @@ async function prerenderData(
await fse.ensureDir(path.dirname(outfile));
await fse.outputFile(outfile, data);
viteConfig.logger.info(`Prerender: Generated ${colors.bold(outfile)}`);
return data;
}

async function prerenderRoute(
Expand Down
13 changes: 1 addition & 12 deletions packages/react-router-serve/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,7 @@ async function run() {
maxAge: "1y",
})
);
app.use(
build.publicPath,
express.static(build.assetsBuildDirectory, {
// Don't redirect directory index.html request to include a trailing slash
redirect: false,
setHeaders: function (res, path) {
if (path.endsWith(".data")) {
res.set("Content-Type", "text/x-turbo");
}
},
})
);
app.use(build.publicPath, express.static(build.assetsBuildDirectory));
app.use(express.static("public", { maxAge: "1h" }));
app.use(morgan("tiny"));

Expand Down
39 changes: 37 additions & 2 deletions packages/react-router/lib/server-runtime/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import type {
LoaderFunctionArgs,
ServerRouteModule,
} from "./routeModules";
import type {
SingleFetchResult,
SingleFetchResults,
} from "../dom/ssr/single-fetch";
import { decodeViaTurboStream } from "../dom/ssr/single-fetch";
import invariant from "./invariant";

export interface RouteManifest<Route> {
[routeId: string]: Route;
Expand Down Expand Up @@ -95,8 +101,37 @@ export function createStaticHandlerDataRoutes(
// Need to use RR's version in the param typed here to permit the optional
// context even though we know it'll always be provided in remix
loader: route.module.loader
? (args: RRLoaderFunctionArgs) =>
callRouteHandler(route.module.loader!, args as LoaderFunctionArgs)
? async (args: RRLoaderFunctionArgs) => {
// If we're prerendering, use the data passed in from prerendering
// the .data route so we dom't call loaders twice
if (args.request.headers.has("X-React-Router-Prerender-Data")) {
let encoded = args.request.headers.get(
"X-React-Router-Prerender-Data"
);
invariant(encoded, "Missing prerendered data for route");
let uint8array = new TextEncoder().encode(encoded);
let stream = new ReadableStream({
start(controller) {
controller.enqueue(uint8array);
controller.close();
},
});
let decoded = await decodeViaTurboStream(stream, global);
let data = decoded.value as SingleFetchResults;
invariant(
data && route.id in data,
"Unable to decode prerendered data"
);
let result = data[route.id] as SingleFetchResult;
invariant("data" in result, "Unable to process prerendered data");
return result.data;
}
let val = await callRouteHandler(
route.module.loader!,
args as LoaderFunctionArgs
);
return val;
}
: undefined,
action: route.module.action
? (args: RRActionFunctionArgs) =>
Expand Down