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): remove external files dependencies #10885

Merged
merged 21 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 15 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ yarn-error.log*
/server/*.js.map
/ssr/dist/
/ssr/*.js
!/ssr/mozilla.dnthelper.min.js
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the file moved.

!/ssr/webpack.config.js
/ssr/*.js.map
/tool/*.js
Expand Down
3 changes: 2 additions & 1 deletion client/config/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ process.env.NODE_PATH = (process.env.NODE_PATH || "")
// injected into the application via DefinePlugin in webpack configuration.
const REACT_APP = /^REACT_APP_/i;

function getClientEnvironment(publicUrl) {
function getClientEnvironment(publicUrl, baseUrl) {
const raw = Object.keys(process.env)
.filter((key) => REACT_APP.test(key))
.reduce(
Expand All @@ -57,6 +57,7 @@ function getClientEnvironment(publicUrl) {
// This should only be used as an escape hatch. Normally you would put
// images into the `src` and `import` them in code to get their paths.
PUBLIC_URL: publicUrl,
BASE_URL: baseUrl,
// We support configuring the sockjs pathname during development.
// These settings let a developer run multiple simultaneous projects.
// They are used as the connection `hostname`, `pathname` and `port`
Expand Down
7 changes: 7 additions & 0 deletions client/config/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ const publicUrlOrPath = getPublicUrlOrPath(
process.env.PUBLIC_URL
);

const baseUrl = getPublicUrlOrPath(
process.env.NODE_ENV === "development",
appPackage.homepage,
process.env.BASE_URL || "/"
);

const buildPath = process.env.BUILD_PATH || "build";

const moduleFileExtensions = [
Expand Down Expand Up @@ -71,6 +77,7 @@ const config = {
appTsBuildInfoFile: resolveApp("../node_modules/.cache/tsconfig.tsbuildinfo"),
swSrc: resolveModule(resolveApp, "src/service-worker"),
publicUrlOrPath,
baseUrl,
libsPath: resolveApp("../libs"),
moduleFileExtensions,
};
Expand Down
5 changes: 4 additions & 1 deletion client/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ function config(webpackEnv) {
// as %PUBLIC_URL% in `index.html` and `process.env.PUBLIC_URL` in JavaScript.
// Omit trailing slash as %PUBLIC_URL%/xyz looks better than %PUBLIC_URL%xyz.
// Get environment variables to inject into our app.
const env = getClientEnvironment(paths.publicUrlOrPath.slice(0, -1));
const env = getClientEnvironment(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather remove trailing slash, than just removing the last char.

paths.publicUrlOrPath.replace(/\/$/, ""),
paths.baseUrl.replace(/\/$/, "")
);

const shouldUseReactRefresh = env.raw.FAST_REFRESH;

Expand Down
8 changes: 4 additions & 4 deletions client/public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
of the file that has a hash in it.
-->

<link rel="icon" href="%PUBLIC_URL%/favicon-48x48.png" />
<link rel="icon" href="/favicon-48x48.png" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to be safe. Those are relative right now because %PUBLIC_URL% does not work. So we remove them until we want to make them absolute.


<link rel="apple-touch-icon" href="%PUBLIC_URL%/apple-touch-icon.png" />
<link rel="apple-touch-icon" href="/apple-touch-icon.png" />

<meta name="theme-color" content="#ffffff" />

<link rel="manifest" href="%PUBLIC_URL%/manifest.json" />
<link rel="manifest" href="/manifest.json" />

<link
rel="search"
Expand Down Expand Up @@ -58,7 +58,7 @@
property="og:description"
content="The MDN Web Docs site provides information about Open Web technologies including HTML, CSS, and APIs for both Web sites and progressive web apps."
/>
<meta property="og:image" content="%PUBLIC_URL%/mdn-social-share.png" />
<meta property="og:image" content="%BASE_URL%/mdn-social-share.png" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using a new %BASE_URL% here because fixing %PUBLIC_URL% is tricky and dangerous.

<meta property="og:image:type" content="image/png" />
<meta property="og:image:height" content="1080" />
<meta property="og:image:width" content="1920" />
Expand Down
12 changes: 12 additions & 0 deletions client/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import printBuildError from "react-dev-utils/printBuildError.js";

import configFactory from "../config/webpack.config.js";
import paths from "../config/paths.js";
import { hashSomeStaticFilesForClientBuild } from "./postprocess-client-build.js";

// Makes the script crash on unhandled rejections instead of silently
// ignoring them. In the future, promise rejections that are not handled will
Expand Down Expand Up @@ -120,6 +121,17 @@ checkBrowsers(paths.appPath, isInteractive)
}
}
)
.then(async () => {
const { results } = await hashSomeStaticFilesForClientBuild(paths.appBuild);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now where the old "optimizeClientBuild" runs. Just after we build the client.

console.log(
chalk.green(
`Hashed ${results.length} files in ${path.join(
paths.appBuild,
"index.html"
)}`
)
);
})
.catch((err) => {
if (err && err.message) {
console.log(err.message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import path from "node:path";
import cheerio from "cheerio";
import md5File from "md5-file";

export async function runOptimizeClientBuild(buildRoot) {
export async function hashSomeStaticFilesForClientBuild(buildRoot) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this optimized in the past. Now it merely hashes files.

const indexHtmlFilePath = path.join(buildRoot, "index.html");
const indexHtml = fs.readFileSync(indexHtmlFilePath, "utf-8");

Expand All @@ -28,15 +28,6 @@ export async function runOptimizeClientBuild(buildRoot) {
}
href = element.attribs.content;
attributeKey = "content";
// This is an unfortunate hack. The value for the
// <meta property=og:image content=...> needs to be an absolute URL.
// We tested with a relative URL and it seems it doesn't work in Twitter.
// So we hardcode the URL to be our production domain so the URL is
// always absolute.
// Yes, this makes it a bit weird to use a build of this on a dev,
// stage, preview, or a local build. Especially if the hashed URL doesn't
// always work. But it's a fair price to pay.
hrefPrefix = "https://developer.mozilla.org";
} else {
href = element.attribs.href;
if (!href) {
Expand Down Expand Up @@ -75,7 +66,7 @@ export async function runOptimizeClientBuild(buildRoot) {
const splitName = filePath.split(extName);
const hashedFilePath = `${splitName[0]}.${hash}${extName}`;
fs.copyFileSync(filePath, hashedFilePath);
const hashedHref = filePathToHref(buildRoot, hashedFilePath);
const hashedHref = filePathToHref(buildRoot, hashedFilePath, href);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ad the href to restore the domain.

results.push({
filePath,
href,
Expand All @@ -101,8 +92,19 @@ export async function runOptimizeClientBuild(buildRoot) {
}

// Turn 'C:\Path\to\client\build\favicon.ico' to '/favicon.ico'
function filePathToHref(root, filePath) {
return `${filePath.replace(root, "").replace(path.sep, "/")}`;
// or 'https://foo.bar/favicon.ico' if href is an absolute URL.
function filePathToHref(root, filePath, href) {
let dummyOrExistingUrl = new URL(href, "http://localhost.example");
dummyOrExistingUrl.pathname = "";
let url = new URL(
`${filePath.replace(root, "").replace(path.sep, "/")}`,
dummyOrExistingUrl
);
if (url.hostname === "localhost.example") {
return url.pathname;
} else {
return url.href;
}
}

// Turn '/favicon.ico' to 'C:\Path\to\client\build\favicon.ico'
Expand Down
5 changes: 4 additions & 1 deletion client/scripts/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ process.on("unhandledRejection", (err) => {

const appPackageJson = JSON.parse(fs.readFileSync(paths.appPackageJson));

const env = getClientEnvironment(paths.publicUrlOrPath.slice(0, -1));
const env = getClientEnvironment(
paths.publicUrlOrPath.replace(/\/$/, ""),
paths.baseUrl.replace(/\/$/, "")
);
const useYarn = fs.existsSync(paths.yarnLockFile);
const isInteractive = process.stdout.isTTY;

Expand Down
4 changes: 1 addition & 3 deletions client/src/homepage/contributor-spotlight/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import { Icon } from "../../ui/atoms/icon";
import Mandala from "../../ui/molecules/mandala";

import "./index.scss";
const contributorGraphic = `${
process.env.PUBLIC_URL || ""
}/assets/mdn_contributor.png`;
const contributorGraphic = "/assets/mdn_contributor.png";

export function ContributorSpotlight(props: HydrationData<any>) {
const fallbackData = props.hyData ? props : undefined;
Expand Down
2 changes: 1 addition & 1 deletion client/src/ui/atoms/avatar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export const Avatar = ({ userData }: { userData: UserData }) => {
// If we have user data and the user is logged in, show their
// profile pic, defaulting to the dino head if the avatar
// URL doesn't work.
const avatarImage = `${process.env.PUBLIC_URL || ""}/assets/avatar.png`;
const avatarImage = "/assets/avatar.png";

return (
<div
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@
"build:curriculum": "cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node build/build-curriculum.ts",
"build:dist": "tsc -p tsconfig.dist.json",
"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 optimize-client-build && yarn tool google-analytics-code && yarn tool popularities && yarn tool spas && yarn tool gather-git-history && yarn tool build-robots-txt",
"build:ssr": "cd ssr && webpack --mode=production",
"build:prepare": "yarn build:client && yarn build:ssr && yarn tool popularities && yarn tool spas && yarn tool gather-git-history && yarn tool build-robots-txt",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

optimize-client-build moved into the client build step.
google-analytics-code moved to SSR prepare.

"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: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'",
"dev": "yarn build:prepare && nf -j Procfile.dev start",
"eslint": "eslint .",
"filecheck": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node filecheck/cli.ts",
"ga": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node ./tool/ga.ts",
"install:all": "find . -mindepth 2 -name 'yarn.lock' ! -wholename '**/node_modules/**' -print0 | xargs -n1 -0 sh -cx 'yarn --cwd $(dirname $0) install'",
"install:all:npm": "find . -mindepth 2 -name 'package-lock.json' ! -wholename '**/node_modules/**' -print0 | xargs -n1 -0 sh -cx 'npm --prefix $(dirname $0) install'",
"jest": "node --experimental-vm-modules --expose-gc ./node_modules/.bin/jest --logHeapUsage",
Expand Down
1 change: 1 addition & 0 deletions ssr/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include.ts
47 changes: 47 additions & 0 deletions ssr/ga.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import fs from "node:fs";
import {
BUILD_OUT_ROOT,
GOOGLE_ANALYTICS_MEASUREMENT_ID,
} from "../libs/env/index.js";
import path from "node:path";

export async function generateGA() {
const outFile = path.join(BUILD_OUT_ROOT, "static", "js", "gtag.js");
const measurementIds =
GOOGLE_ANALYTICS_MEASUREMENT_ID.split(",").filter(Boolean);
if (measurementIds.length) {
const dntHelperCode = fs
.readFileSync(
new URL("mozilla.dnthelper.min.js", import.meta.url),
"utf-8"
)
.trim();

const firstMeasurementId = measurementIds[0];
const gaScriptURL = `https://www.googletagmanager.com/gtag/js?id=${encodeURIComponent(firstMeasurementId)}`;

const code = `
// Mozilla DNT Helper
${dntHelperCode}
// Load GA unless DNT is enabled.
if (Mozilla && !Mozilla.dntEnabled()) {
window.dataLayer = window.dataLayer || [];
function gtag(){dataLayer.push(arguments);}
gtag('js', new Date());
${measurementIds
.map((id) => `gtag('config', '${id}', { 'anonymize_ip': true });`)
.join("\n ")}

var gaScript = document.createElement('script');
gaScript.async = true;
gaScript.src = '${gaScriptURL}';
document.head.appendChild(gaScript);
}`.trim();
fs.writeFileSync(outFile, `${code}\n`, "utf-8");
console.log(
`Generated ${outFile} for SSR rendering using ${GOOGLE_ANALYTICS_MEASUREMENT_ID}.`
);
} else {
console.log("No Google Analytics code file generated");
}
}
4 changes: 4 additions & 0 deletions ssr/include.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const WEBFONT_TAGS: string;
export const GTAG_PATH: null | string;
export const BASE_URL: string;
export const ALWAYS_ALLOW_ROBOTS: boolean;
10 changes: 0 additions & 10 deletions ssr/index.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
import path from "node:path";
import { fileURLToPath } from "node:url";

import * as dotenv from "dotenv";
import React from "react";
import { StaticRouter } from "react-router-dom/server";

import { App } from "../client/src/app";
import render from "./render";

const dirname = fileURLToPath(new URL(".", import.meta.url));

dotenv.config({
path: path.join(dirname, "..", process.env.ENV_FILE || ".env"),
});

export function renderHTML(url, context) {
return render(
React.createElement(
Expand Down
File renamed without changes.
73 changes: 73 additions & 0 deletions ssr/prepare.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted code from ssr/render.ts

Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
import {
ALWAYS_ALLOW_ROBOTS,
BUILD_OUT_ROOT,
BASE_URL,
} from "../libs/env/index.js";
import { generateGA } from "./ga.js";

const dirname = path.dirname(fileURLToPath(new URL(".", import.meta.url)));
const clientBuildRoot = path.resolve(dirname, "client/build");

function extractWebFontURLs() {
const urls: string[] = [];
const manifest = JSON.parse(
fs.readFileSync(path.join(clientBuildRoot, "asset-manifest.json"), "utf-8")
);
for (const entrypoint of manifest.entrypoints) {
if (!entrypoint.endsWith(".css")) continue;
const css = fs.readFileSync(
path.join(clientBuildRoot, entrypoint),
"utf-8"
);
const generator = extractCSSURLs(css, (url) => url.endsWith(".woff2"));
urls.push(...generator);
}
return [...new Set(urls)];
}

function* extractCSSURLs(css, filterFunction) {
for (const match of css.matchAll(/url\((.*?)\)/g)) {
const url = match[1];
if (filterFunction(url)) {
yield url;
}
}
}

function webfontTags(webfontURLs): string {
return webfontURLs
.map(
(url) =>
`<link rel="preload" as="font" type="font/woff2" href="${url}" crossorigin>`
)
.join("");
}

function gtagScriptPath(relPath = "/static/js/gtag.js") {
const filePath = relPath.split("/").slice(1).join(path.sep);
if (fs.existsSync(path.join(BUILD_OUT_ROOT, filePath))) {
return relPath;
}
return null;
}

function prepare() {
const webfontURLs = extractWebFontURLs();
const tags = webfontTags(webfontURLs);
const gtagPath = gtagScriptPath();

fs.writeFileSync(
path.join(dirname, "ssr", "include.ts"),
`
export const WEBFONT_TAGS = ${JSON.stringify(tags)};
export const GTAG_PATH = ${JSON.stringify(gtagPath)};
export const BASE_URL = ${JSON.stringify(BASE_URL)};
export const ALWAYS_ALLOW_ROBOTS = ${JSON.stringify(ALWAYS_ALLOW_ROBOTS)};
`
);
}

generateGA().then(() => prepare());
Loading
Loading