Skip to content
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

feat(ssr): render in Cloud Function, not in build #11459

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
3 changes: 0 additions & 3 deletions .github/workflows/dev-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,6 @@ jobs:
# Generate sitemap index file
yarn build --sitemap-index

# SSR all pages
yarn render:html

# Generate whatsdeployed files.
yarn tool whatsdeployed --output client/build/_whatsdeployed/code.json
yarn tool whatsdeployed $CONTENT_ROOT --output client/build/_whatsdeployed/content.json
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/prod-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,6 @@ jobs:
# Generate sitemap index file
yarn build --sitemap-index

# SSR all pages
yarn render:html

# Generate whatsdeployed files.
yarn tool whatsdeployed --output client/build/_whatsdeployed/code.json
yarn tool whatsdeployed $CONTENT_ROOT --output client/build/_whatsdeployed/content.json
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/stage-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,6 @@ jobs:
# Generate sitemap index file
yarn build --sitemap-index

# SSR all pages
yarn render:html

# Generate whatsdeployed files.
yarn tool whatsdeployed --output client/build/_whatsdeployed/code.json
yarn tool whatsdeployed $CONTENT_ROOT --output client/build/_whatsdeployed/content.json
Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/test-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,6 @@ jobs:
# Generate sitemap index file
yarn build --sitemap-index

# SSR all pages
yarn render:html

# Generate whatsdeployed files.
yarn tool whatsdeployed --output client/build/_whatsdeployed/code.json
yarn tool whatsdeployed $CONTENT_ROOT --output client/build/_whatsdeployed/content.json
Expand Down Expand Up @@ -240,6 +237,7 @@ jobs:
run: |
npm ci
npm run build-redirects
npm run build-canonicals

- name: Deploy Function
if: ${{ ! vars.SKIP_FUNCTION }}
Expand Down
3 changes: 2 additions & 1 deletion cloud-function/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
"build-canonicals": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node src/build-canonicals.ts",
"build-redirects": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node src/build-redirects.ts",
"copy-internal": "rm -rf ./src/internal && cp -R ../libs ./src/internal",
"copy-ssr": "mkdir -p ./src/internal/ssr && cp ../ssr/dist/main.js ./src/internal/ssr/",
LeoMcA marked this conversation as resolved.
Show resolved Hide resolved
"gcp-build": "npm run build",
"prepare": "([ ! -e ../libs ] || npm run copy-internal)",
"prepare": "([ ! -e ../libs ] || npm run copy-internal) && ([ ! -e ../ssr/dist ] || npm run copy-ssr)",
"proxy": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node src/proxy.ts",
"server": "npm run build && functions-framework --target=mdnHandler",
"server:watch": "nodemon --exec npm run server",
Expand Down
4 changes: 4 additions & 0 deletions cloud-function/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { resolveRunnerHtml } from "./middlewares/resolve-runner-html.js";
import { proxyRunner } from "./handlers/proxy-runner.js";
import { stripForwardedHostHeaders } from "./middlewares/stripForwardedHostHeaders.js";
import { proxyPong } from "./handlers/proxy-pong.js";
import { handleRenderHTML } from "./handlers/render-html.js";

const router = Router();
router.use(stripForwardedHostHeaders);
Expand Down Expand Up @@ -88,6 +89,7 @@ router.get(
redirectTrailingSlash,
redirectMovedPages,
resolveIndexHTML,
handleRenderHTML,
proxyContent
);
router.get(
Expand All @@ -96,6 +98,7 @@ router.get(
redirectLocale,
redirectEnforceTrailingSlash,
resolveIndexHTML,
handleRenderHTML,
proxyContent
);
// MDN Plus, static pages, etc.
Expand All @@ -106,6 +109,7 @@ router.get(
redirectLocale,
redirectTrailingSlash,
resolveIndexHTML,
handleRenderHTML,
proxyContent
);
router.all("*", notFound);
Expand Down
27 changes: 8 additions & 19 deletions cloud-function/src/handlers/proxy-content.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
/* eslint-disable n/no-unsupported-features/node-builtins */
import {
createProxyMiddleware,
fixRequestBody,
responseInterceptor,
} from "http-proxy-middleware";

import { withContentResponseHeaders } from "../headers.js";
import { withProxiedContentResponseHeaders } from "../headers.js";
import { Source, sourceUri } from "../env.js";
import { PROXY_TIMEOUT } from "../constants.js";
import { isLiveSampleURL } from "../utils.js";

const NOT_FOUND_PATH = "en-us/_spas/404.html";

let notFoundBuffer: ArrayBuffer;
import { renderHTMLForContext } from "./render-html.js";

const target = sourceUri(Source.content);

Expand All @@ -27,23 +23,16 @@ export const proxyContent = createProxyMiddleware({
proxyReq: fixRequestBody,
proxyRes: responseInterceptor(
async (responseBuffer, proxyRes, req, res) => {
withContentResponseHeaders(proxyRes, req, res);
if (proxyRes.statusCode === 404 && !isLiveSampleURL(req.url ?? "")) {
const tryHtml = await fetch(
`${target}${req.url?.slice(1)}/index.html`
const html = await renderHTMLForContext(
req,
res,
`${target}${req.url?.slice(1)}/index.json`
);
if (tryHtml.ok) {
res.statusCode = 200;
res.setHeader("Content-Type", "text/html");
return Buffer.from(await tryHtml.arrayBuffer());
} else if (!notFoundBuffer) {
const response = await fetch(`${target}${NOT_FOUND_PATH}`);
notFoundBuffer = await response.arrayBuffer();
}
res.setHeader("Content-Type", "text/html");
return Buffer.from(notFoundBuffer);
return Buffer.from(html);
}

withProxiedContentResponseHeaders(proxyRes, req, res);
return responseBuffer;
}
),
Expand Down
52 changes: 52 additions & 0 deletions cloud-function/src/handlers/render-html.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/* eslint-disable n/no-unsupported-features/node-builtins */
import type { NextFunction, Request, Response } from "express";
import type { IncomingMessage, ServerResponse } from "node:http";

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
import { renderHTML } from "../internal/ssr/main.js";
import { sourceUri, Source } from "../env.js";
import { withRenderedContentResponseHeaders } from "../headers.js";

const target = sourceUri(Source.content);

export async function handleRenderHTML(
LeoMcA marked this conversation as resolved.
Show resolved Hide resolved
req: Request,
res: Response,
next: NextFunction
) {
if (req.url.endsWith("/index.html")) {
const html = await renderHTMLForContext(
req,
res,
target.replace(/\/$/, "") + req.url.replace(/html$/, "json")
);
res.send(html).end();
} else {
next();
}
}

export async function renderHTMLForContext(
req: IncomingMessage,
res: ServerResponse<IncomingMessage>,
contextUrl: string
) {
res.setHeader("Content-Type", "text/html");
res.setHeader("X-MDN-SSR", "true");
try {
const contextRes = await fetch(contextUrl);
if (!contextRes.ok) {
throw new Error(contextRes.status.toString());
}
const context = await contextRes.json();
res.statusCode = 200;
withRenderedContentResponseHeaders(req, res);
return renderHTML(context);
} catch {
res.statusCode = 404;
LeoMcA marked this conversation as resolved.
Show resolved Hide resolved
withRenderedContentResponseHeaders(req, res);
const context = { url: req.url, pageNotFound: true };
return renderHTML(context);
}
}
25 changes: 24 additions & 1 deletion cloud-function/src/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const NO_CACHE_VALUE = "no-store, must-revalidate";

const HASHED_REGEX = /\.[a-f0-9]{8,32}\./;

export function withContentResponseHeaders(
export function withProxiedContentResponseHeaders(
proxyRes: IncomingMessage,
req: IncomingMessage,
res: ServerResponse<IncomingMessage>
Expand Down Expand Up @@ -56,6 +56,29 @@ export function withContentResponseHeaders(
return res;
}

export function withRenderedContentResponseHeaders(
req: IncomingMessage,
res: ServerResponse<IncomingMessage>
) {
if (res.headersSent) {
console.warn(
`Cannot set content response headers. Headers already sent for: ${req.url}`
);
return;
}

const url = req.url ?? "";

setContentResponseHeaders((name, value) => res.setHeader(name, value), {});

const cacheControl = getCacheControl(res.statusCode ?? 0, url);
if (cacheControl) {
res.setHeader("Cache-Control", cacheControl);
}

return res;
}

function getCacheControl(statusCode: number, url: string) {
if (
statusCode === 404 ||
Expand Down
3 changes: 1 addition & 2 deletions cloud-function/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
"noPropertyAccessFromIndexSignature": true,
"noUncheckedIndexedAccess": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"checkJs": true
LeoMcA marked this conversation as resolved.
Show resolved Hide resolved
"noUnusedParameters": true
},
"ts-node": {
"esm": true,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"build:docs": "cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node build/cli.ts -n",
"build:glean": "cd client && cross-env VIRTUAL_ENV=venv glean translate src/telemetry/metrics.yaml src/telemetry/pings.yaml -f typescript -o src/telemetry/generated",
"build:prepare": "yarn build:client && yarn build:ssr && yarn tool popularities && yarn tool spas && yarn tool gather-git-history && yarn tool build-robots-txt",
"build:ssr": "cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node ssr/prepare.ts && cd ssr && webpack --mode=production",
"build:ssr": "cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node ssr/prepare.ts && cd ssr && cross-env NODE_ENV=production webpack --mode=production",
"build:sw": "cd client/pwa && yarn && yarn build:prod",
"build:sw-dev": "cd client/pwa && yarn && yarn build",
"check:tsc": "find . -name 'tsconfig.json' ! -wholename '**/node_modules/**' -print0 | xargs -n1 -P 2 -0 sh -c 'cd `dirname $0` && echo \"🔄 $(pwd)\" && npx tsc --noEmit && echo \"☑️ $(pwd)\" || exit 255'",
Expand Down
4 changes: 4 additions & 0 deletions ssr/webpack.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { fileURLToPath } from "node:url";
import nodeExternals from "webpack-node-externals";
import webpack from "webpack";
import getClientEnvironment from "../client/config/env.js";

const env = getClientEnvironment();

const config = {
context: fileURLToPath(new URL(".", import.meta.url)),
Expand Down Expand Up @@ -94,6 +97,7 @@ const config = {
new webpack.optimize.LimitChunkCountPlugin({
maxChunks: 1,
}),
new webpack.DefinePlugin(env.stringified),
],
experiments: {
outputModule: true,
Expand Down