feat(ssr): remove external files dependencies #10885

fiji-flo · 2024-04-09T14:23:34Z

Summary

Before the ssr dist had dependencies on external files (e.g. index.html).

By in-lining those dependencies we can generate a portable standalone main.js.

This also fixed our font preloading logic.

Note

This leaves most of the code as it was and we'll simplify it later. Just to minimize the risk.

Problem

Using the SSR main.js outside of yari failed because of external dependencies.

Solution

Add a build script and inline dependencies.

How did you test this change?

Locally and about to deploy to stage.

Before the ssr dist had dependencies on external files (e.g. index.html). By in-lining those dependencies we can generate a portable standalone main.js.

Also fix PUBLIC_URL support

fiji-flo · 2024-04-11T14:49:16Z

.gitignore

@@ -47,6 +47,7 @@ yarn-error.log*
 /server/*.js.map
 /ssr/dist/
 /ssr/*.js
+!/ssr/mozilla.dnthelper.min.js


Since the file moved.

fiji-flo · 2024-04-11T14:52:46Z

client/config/paths.js

+    process.env.BASE_URL ||
+    "https://developer.mozilla.org/"


Add fallback(s). We should replace PUBLIC_URL everywhere, but that's to much for this PR:

fiji-flo · 2024-04-11T14:54:10Z

client/public/index.html

@@ -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" />


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.

fiji-flo · 2024-04-11T14:55:05Z

client/scripts/build.js

@@ -120,6 +121,17 @@ checkBrowsers(paths.appPath, isInteractive)
      }
    }
  )
+  .then(async () => {
+    const { results } = await hashSomeStaticFilesForClientBuild(paths.appBuild);


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

fiji-flo · 2024-04-11T14:55:26Z

client/scripts/postprocess-client-build.js

@@ -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);


Ad the href to restore the domain.

fiji-flo · 2024-04-22T12:32:09Z

client/config/webpack.config.js

@@ -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(


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

fiji-flo · 2024-04-22T12:32:54Z

client/public/index.html

@@ -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" />


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

fiji-flo · 2024-04-22T12:33:36Z

client/scripts/postprocess-client-build.js

@@ -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) {


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

fiji-flo · 2024-04-22T12:39:49Z

package.json

@@ -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",


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

fiji-flo · 2024-04-22T12:40:40Z

ssr/prepare.ts

Extracted code from ssr/render.ts

fiji-flo · 2024-04-22T12:41:19Z

ssr/webpack.config.js

@@ -23,6 +23,10 @@ const config = {
  },
  module: {
    rules: [
+      {
+        resourceQuery: /raw/,


Allows us to include the post processed index.html as string.

LeoMcA

Did a local diff between this and main, saw no differences other than those which were intentional. Looks good on staging.

Just one leftover console.log:

testing/tests/index.test.ts

feat(ssr): remove external files dependencies

38e51ae

Before the ssr dist had dependencies on external files (e.g. index.html). By in-lining those dependencies we can generate a portable standalone main.js.

github-actions bot added the dependencies Pull requests that update a dependency file label Apr 9, 2024

fiji-flo added 11 commits April 9, 2024 23:29

build gtag before ssr

e31f711

extract ga generation

4722a8f

move ga generation to ssr

3f53f72

use asset-loader and ignore ts and esling error

8568ff7

add include.d.ts

da96be8

fixes

e4ab749

remove console.log

3a7feb4

optimize client

74a488b

move optimize client to build step

9fb44aa

Also fix PUBLIC_URL support

fix test by adding default

00346b7

comment

7c80156

fiji-flo commented Apr 11, 2024

View reviewed changes

fiji-flo added 4 commits April 12, 2024 10:18

lint

8d3f145

don't touch PUBLIC_URL

7ffbf3b

Merge remote-tracking branch 'upstream/main' into ssr2.0

616a17d

remove ga step

593d82e

fiji-flo commented Apr 22, 2024

View reviewed changes

fiji-flo marked this pull request as ready for review April 22, 2024 12:41

fiji-flo requested review from mdn-bot and a team as code owners April 22, 2024 12:41

fiji-flo requested a review from argl April 22, 2024 12:42

fiji-flo added 3 commits April 22, 2024 15:18

hack base url

cdee340

fix index.html

4e5e120

fix server

3e77fc2

LeoMcA requested review from LeoMcA and removed request for argl April 23, 2024 10:24

LeoMcA self-assigned this Apr 23, 2024

This comment was marked as outdated.

Sign in to view

LeoMcA approved these changes Apr 24, 2024

View reviewed changes

testing/tests/index.test.ts Outdated Show resolved Hide resolved

fiji-flo added 2 commits April 25, 2024 11:34

remove log

0af8d62

Merge branch 'main' into ssr2.0

c1708e3

fiji-flo merged commit 8fea595 into main Apr 25, 2024
16 checks passed

fiji-flo deleted the ssr2.0 branch April 25, 2024 10:58

github-actions bot mentioned this pull request Apr 25, 2024

chore(main): release 2.50.0 #10956

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ssr): remove external files dependencies #10885

feat(ssr): remove external files dependencies #10885

fiji-flo commented Apr 9, 2024 •

edited

Loading

fiji-flo Apr 11, 2024

fiji-flo Apr 11, 2024

fiji-flo Apr 11, 2024

fiji-flo Apr 11, 2024

fiji-flo Apr 11, 2024

fiji-flo Apr 22, 2024

fiji-flo Apr 22, 2024

fiji-flo Apr 22, 2024

fiji-flo Apr 22, 2024

fiji-flo Apr 22, 2024

fiji-flo Apr 22, 2024

This comment was marked as outdated.

LeoMcA left a comment

feat(ssr): remove external files dependencies #10885

feat(ssr): remove external files dependencies #10885

Conversation

fiji-flo commented Apr 9, 2024 • edited Loading

Summary

Problem

Solution

How did you test this change?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

LeoMcA left a comment

Choose a reason for hiding this comment

fiji-flo commented Apr 9, 2024 •

edited

Loading