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.

Sign up for GitHub

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

Jump to bottom

feat(ssr): render JSX instead of index.html #11672

Merged
merged 17 commits into from
Sep 16, 2024
Merged

feat(ssr): render JSX instead of index.html #11672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
19a1854
chore(ssr): rename render.ts to render.tsx
LeoMcA Aug 21, 2024
96fd148
chore(ssr): use jsx in ssr/render.tsx
LeoMcA Aug 21, 2024
3f27fe5
fix(ssr): test for asset-manifest.json in yarn start
LeoMcA Aug 22, 2024
03ac9fe
fix(ssr): weird path resolution in libs/env breaking npm-published-si…
LeoMcA Aug 22, 2024
84b8942
fix(ssr): fix failing index.html tests
LeoMcA Aug 22, 2024
73fe2c2
fix(ssr): handle absolute paths in libs/env
LeoMcA Aug 22, 2024
6730b61
fix tests
LeoMcA Aug 22, 2024
80abd8b
revert changes to libs/env, use actual client webpack outputPath
LeoMcA Aug 22, 2024
1285538
🙏
LeoMcA Aug 22, 2024
2f35d67
webpack-node-externals wasn't working, but now it works things break,…
LeoMcA Aug 22, 2024
49c5c46
fix tests
LeoMcA Aug 22, 2024
abec174
Merge remote-tracking branch 'origin/main' into ssr-render-jsx
LeoMcA Sep 6, 2024
bd4279c
Merge remote-tracking branch 'origin/main' into ssr-render-jsx
LeoMcA Sep 6, 2024
53da59c
Merge remote-tracking branch 'origin/main' into ssr-render-jsx
LeoMcA Sep 10, 2024
cc97a91
Merge remote-tracking branch 'origin/main' into ssr-render-jsx
LeoMcA Sep 10, 2024
7c2a1bc
wip(ssr): review updates
LeoMcA Sep 10, 2024
afaa1cc
Merge remote-tracking branch 'origin/main' into ssr-render-jsx
LeoMcA Sep 16, 2024
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
3 changes: 0 additions & 3 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ yarn-error.log*
/server/*.js
/server/*.js.map
/ssr/dist/
/ssr/*.js
!/ssr/mozilla.dnthelper.min.js
!/ssr/webpack.config.js
/ssr/*.js.map
/tool/*.js
/tool/*.js.map
Expand Down
35 changes: 10 additions & 25 deletions client/config/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,31 +383,16 @@ function config(webpackEnv) {
},
plugins: [
// Generates an `index.html` file with the <script> injected.
new HtmlWebpackPlugin(
Object.assign(
{},
{
inject: true,
template: paths.appHtml,
},
isEnvProduction
? {
minify: {
removeComments: true,
collapseWhitespace: true,
removeRedundantAttributes: true,
useShortDoctype: true,
removeEmptyAttributes: true,
removeStyleLinkTypeAttributes: true,
keepClosingSlash: true,
minifyJS: true,
minifyCSS: true,
minifyURLs: true,
},
}
: undefined
)
LeoMcA marked this conversation as resolved.
Show resolved Hide resolved
),
isEnvDevelopment &&
new HtmlWebpackPlugin(
Object.assign(
{},
{
inject: true,
template: paths.appHtml,
}
)
),
// Makes some environment variables available to the JS code, for example:
// if (process.env.NODE_ENV === 'production') { ... }. See `./env.js`.
// It is absolutely essential that NODE_ENV is set to production
Expand Down
126 changes: 1 addition & 125 deletions client/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,136 +1,12 @@
<!doctype html>
<html lang="en" prefix="og: https://ogp.me/ns#">
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />

<!--
Note that our build process will rewrite this 'href' value to a copy
of the file that has a hash in it.
-->

<link rel="icon" href="/favicon-48x48.png" />

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

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

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

<link
rel="search"
type="application/opensearchdescription+xml"
href="/opensearch.xml"
title="MDN Web Docs"
/>

<title>MDN Web Docs</title>
<meta name="SSR_DATA" />
<meta
name="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."
/>

<!--
Why these Open Graph meta tags? ...when the '<title>' and '<meta name=description>'
already exist?
Well, because of Twitter.
When you paste in an MDN URL into Twitter it will not make a "card".
See this comment: https://github.com/mdn/yari/issues/3590#issuecomment-825153396
LeoMcA marked this conversation as resolved.
Show resolved Hide resolved

According to Twitter's documentation on "Cards" they will use their proprietary
tags (e.g. `<meta name="twitter:card" content="summary">`) but if not available
they will fall back to Open Graph meta tags.
See https://developer.twitter.com/en/docs/twitter-for-websites/cards/guides/getting-started#opengraph

It is a lot of repetition and additional bloat to the index.html files. But
keep in mind that repeated strings of texts compresses very well and most
clients will only download the Gzip or Brotli compressed HTML file.

Remember, all of these are *default* values. The actual values are processed in
ssr/render.js and uses cheerio to replace the content on all of these.
-->
<meta property="og:url" content="https://developer.mozilla.org" />
<meta property="og:title" content="MDN Web Docs" />
<meta property="og:type" content="website" />
<meta property="og:locale" content="en_US" />
<meta
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="/mdn-social-share.png" />
<meta property="og:image:type" content="image/png" />
<meta property="og:image:height" content="1080" />
<meta property="og:image:width" content="1920" />
<meta
property="og:image:alt"
content="The MDN Web Docs logo, featuring a blue accent color, displayed on a solid black background."
/>
<meta property="og:site_name" content="MDN Web Docs" />
<meta name="twitter:card" content="summary_large_image" />
<meta name="twitter:creator" content="MozDevNet" />

<link rel="canonical" href="https://developer.mozilla.org" />
<style media="print">
.article-actions-container,
.main-menu-toggle,
.document-toc-container,
.on-github,
.sidebar,
.top-navigation-main,
.page-footer,
.top-banner,
.place,
ul.prev-next,
.language-menu {
display: none !important;
}

.main-page-content,
.main-page-content pre {
padding: 2px;
}

.main-page-content pre {
border-left-width: 2px;
}
</style>
<meta name="SSR_SCRIPTS" />
</head>

<body>
<script>
/**
* If we modify this script, we must update the CSP hash as follows:
* 1. Run `yarn run jest testing/tests/csp.test.ts --watch`
* 2. Open `libs/constants/index.js` and find the current hash in CSP_SCRIPT_SRC_VALUES.
* 3. Remove the old "previous" hash and replace it with the old "current" hash.
* 4. Replace the old "current" hash with the new hash from the failing test (step 1).
*/
document.body.addEventListener(
"load",
(e) => {
if (e.target.classList.contains("interactive")) {
e.target.setAttribute("data-readystate", "complete");
}
},
{ capture: true }
);

if (window && document.documentElement) {
const c = { light: "#ffffff", dark: "#1b1b1b" };
try {
const o = window.localStorage.getItem("theme");
o &&
((document.documentElement.className = o),
(document.documentElement.style.backgroundColor = c[o]));
const n = window.localStorage.getItem("nop");
n && (document.documentElement.dataset["nop"] = n);
} catch (e) {
console.warn("Unable to read theme from localStorage", e);
}
}
LeoMcA marked this conversation as resolved.
Show resolved Hide resolved
</script>
<div id="root"></div>
</body>
</html>
13 changes: 0 additions & 13 deletions client/scripts/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
// Ensure environment variables are read.
import "../config/env.js";

import path from "node:path";
import chalk from "chalk";
import fs from "fs-extra";
import webpack from "webpack";

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 @@ -37,17 +35,6 @@ build()
process.exit(1);
}
)
.then(async () => {
const { results } = await hashSomeStaticFilesForClientBuild(paths.appBuild);
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
121 changes: 0 additions & 121 deletions client/scripts/postprocess-client-build.js
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion libs/constants/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const CSP_SCRIPT_SRC_VALUES = [
"https://js.stripe.com",

/*
* Inline scripts (defined in `client/public/index.html`).
* Inline scripts (imported in `ssr/render.tsx`).
*
* If we modify them, we must always update their CSP hash here.
*
Expand Down
8 changes: 5 additions & 3 deletions package.json
View file
Open in desktop
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 && webpack --mode=production --config=ssr/webpack.config.js",
"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 All @@ -41,7 +41,7 @@
"prettier-check": "prettier --check .",
"prettier-format": "prettier --write .",
"render:html": "cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node build/ssr-cli.ts",
"start": "(test -f client/build/index.html || yarn build:client) && (test -f ssr/dist/main.js || yarn build:ssr) && (test -f popularities.json || yarn tool popularities) && (test -d client/build/en-us/_spas || yarn tool spas) && nf -j Procfile.start start",
"start": "(test -f client/build/asset-manifest.json || yarn build:client) && (test -f ssr/dist/main.js || yarn build:ssr) && (test -f popularities.json || yarn tool popularities) && (test -d client/build/en-us/_spas || yarn tool spas) && nf -j Procfile.start start",
"start:client": "cd client && cross-env NODE_ENV=development BABEL_ENV=development PORT=3000 node scripts/start.js",
"start:server": "node-dev --experimental-loader ts-node/esm server/index.ts",
"start:static-server": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node server/static.ts",
Expand All @@ -56,7 +56,7 @@
"test:prepare": "yarn build:prepare && yarn build:docs && yarn render:html && yarn start:static-server",
"test:testing": "yarn jest --rootDir testing",
"tool": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node ./tool/cli.ts",
"watch:ssr": "cd ssr && webpack --mode=production --watch"
"watch:ssr": "webpack --mode=production --watch --config=ssr/webpack.config.js"
},
"resolutions": {
"http-cache-semantics": ">=4.1.1",
Expand Down Expand Up @@ -183,6 +183,7 @@
"cross-env": "^7.0.3",
"css-loader": "^7.1.2",
"css-minimizer-webpack-plugin": "^7.0.0",
"cssnano": "^7.0.6",
"diff": "^7.0.0",
"downshift": "^7.6.1",
"eslint": "^8.57.0",
Expand Down Expand Up @@ -251,6 +252,7 @@
"stylelint-prettier": "^4.1.0",
"stylelint-scss": "^5.3.2",
"swr": "^2.2.5",
"terser-loader": "^2.0.3",
"terser-webpack-plugin": "^5.3.10",
"ts-jest": "^29.2.5",
"ts-loader": "^9.5.1",
Expand Down
Loading