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

LeoMcA · 2024-08-22T11:12:38Z

NB: first commit just renames ssr/render.ts to ssr/render.tsx, easiest to see changes in the second commit (with whitespace changes ignored)

Summary

https://mozilla-hub.atlassian.net/browse/MP-1445

First of many incremental improvements to the client build system.

Problem

We do hard to understand/modify manual string manipulation of a template to SSR our app.
We also do a bunch of weird custom asset handling/hashing/etc. which webpack is exceptionally good at due to our webpack config being previously hidden behind CRA.

Solution

Move our index.html template into ssr/render as JSX.
Generate CSP hash for inline assets in webpack
Remove manual asset hashing of icons etc.
Deliberately kept other changes in ssr/render minimal to reduce diff/make review easier - improvements/simplifications/etc. there are for further iterations

How did you test this change?

git checkout main
yarn && yarn build:prepare
BUILD_FOLDERSEARCH=web/javascript/reference/global_objects/array/foreach yarn build:docs
yarn render:html
mv client/build ../build.main

# repeat for this branch
git checkout ssr-render-jsx
yarn && yarn build:prepare
BUILD_FOLDERSEARCH=web/javascript/reference/global_objects/array/foreach yarn build:docs
yarn render:html
mv client/build ../build.ssr-jsx

# manual diff (vscode gives nice inline diffs)
code --diff ../build.main/en-us/index.html ../build.ssr-jsx/en-us/index.html
code --diff ../build.main/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html ../build.ssr-jsx/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html

# generate usable diffs for below
npx prettier ../build.main/en-us/index.html ../build.ssr-jsx/en-us/index.html ../build.main/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html ../build.ssr-jsx/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html --write
diff -u ../build.main/en-us/index.html ../build.ssr-jsx/en-us/index.html
diff -u ../build.main/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html ../build.ssr-jsx/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html

--- ../build.main/en-us/index.html      2024-08-22 10:50:31.949840758 +0000
+++ ../build.ssr-jsx/en-us/index.html   2024-08-22 10:50:32.069847759 +0000
@@ -3,10 +3,19 @@
   <head>
     <meta charset="utf-8" />
     <meta name="viewport" content="width=device-width,initial-scale=1" />
-    <link rel="icon" href="/favicon-48x48.cbbd161b.png" />
-    <link rel="apple-touch-icon" href="/apple-touch-icon.6803c6f0.png" />
+    <link
+      rel="icon"
+      href="https://developer.mozilla.org/favicon-48x48.bc390275e955dacb2e65.png"
+    />
+    <link
+      rel="apple-touch-icon"
+      href="https://developer.mozilla.org/apple-touch-icon.528534bba673c38049c2.png"
+    />
     <meta name="theme-color" content="#ffffff" />
-    <link rel="manifest" href="/manifest.56b1cedc.json" />
+    <link
+      rel="manifest"
+      href="https://developer.mozilla.org/manifest.f42880861b394dd4dc9b.json"
+    />
     <link
       rel="search"
       type="application/opensearchdescription+xml"
@@ -19,7 +28,7 @@
       as="font"
       type="font/woff2"
       href="/static/media/Inter.var.c2fe3cb2b7c746f7966a.woff2"
-      crossorigin
+      crossorigin=""
     />
     <link
       rel="alternate"
@@ -43,7 +52,7 @@
     />
     <meta
       property="og:image"
-      content="https://developer.mozilla.org/mdn-social-share.cd6c4a5a.png"
+      content="https://developer.mozilla.org/mdn-social-share.d893525a4fb5fb1f67a2.png"
     />
     <meta property="og:image:type" content="image/png" />
     <meta property="og:image:height" content="1080" />
@@ -78,7 +87,7 @@
         border-left-width: 2px;
       }
     </style>
-    <script defer="defer" src="/static/js/main.8ef4b2e8.js"></script>
+    <script defer="" src="/static/js/main.8ef4b2e8.js"></script>
     <link href="/static/css/main.debc7a6f.css" rel="stylesheet" />
   </head>
   <body>
@@ -1066,7 +1075,7 @@
                       ></span
                     >
                   </p>
-                  <span class="request-date">14 minutes ago</span>
+                  <span class="request-date">34 minutes ago</span>
                 </li>
                 <li class="request-item">
                   <p class="request-title">
@@ -1080,7 +1089,7 @@
                       ></span
                     >
                   </p>
-                  <span class="request-date">35 minutes ago</span>
+                  <span class="request-date">an hour ago</span>
                 </li>
                 <li class="request-item">
                   <p class="request-title">

--- ../build.main/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html   2024-08-22 10:50:32.259858842 +0000
+++ ../build.ssr-jsx/en-us/docs/web/javascript/reference/global_objects/array/foreach/index.html        2024-08-22 10:50:32.399867009 +0000
@@ -3,10 +3,19 @@
   <head>
     <meta charset="utf-8" />
     <meta name="viewport" content="width=device-width,initial-scale=1" />
-    <link rel="icon" href="/favicon-48x48.cbbd161b.png" />
-    <link rel="apple-touch-icon" href="/apple-touch-icon.6803c6f0.png" />
+    <link
+      rel="icon"
+      href="https://developer.mozilla.org/favicon-48x48.bc390275e955dacb2e65.png"
+    />
+    <link
+      rel="apple-touch-icon"
+      href="https://developer.mozilla.org/apple-touch-icon.528534bba673c38049c2.png"
+    />
     <meta name="theme-color" content="#ffffff" />
-    <link rel="manifest" href="/manifest.56b1cedc.json" />
+    <link
+      rel="manifest"
+      href="https://developer.mozilla.org/manifest.f42880861b394dd4dc9b.json"
+    />
     <link
       rel="search"
       type="application/opensearchdescription+xml"
@@ -67,7 +76,7 @@
       as="font"
       type="font/woff2"
       href="/static/media/Inter.var.c2fe3cb2b7c746f7966a.woff2"
-      crossorigin
+      crossorigin=""
     />
     <link
       rel="alternate"
@@ -99,7 +108,7 @@
     />
     <meta
       property="og:image"
-      content="https://developer.mozilla.org/mdn-social-share.cd6c4a5a.png"
+      content="https://developer.mozilla.org/mdn-social-share.d893525a4fb5fb1f67a2.png"
     />
     <meta property="og:image:type" content="image/png" />
     <meta property="og:image:height" content="1080" />
@@ -137,7 +146,7 @@
         border-left-width: 2px;
       }
     </style>
-    <script defer="defer" src="/static/js/main.8ef4b2e8.js"></script>
+    <script defer="" src="/static/js/main.8ef4b2e8.js"></script>
     <link href="/static/css/main.debc7a6f.css" rel="stylesheet" />
   </head>
   <body>

As you can see, the changes are minimal:

pre-pending https://developer.mozilla.org to the icon/manifest paths
different hashing algo/length on icon/manifest paths
small html attribute syntax changes, e.g crossorigin -> crossorigin=""
not visible after running through prettier, but visible in the inline diff: self closing some tags which weren't before

generate csp hashes in webpack remove manual asset hashing

…mulation https://github.com/mdn/yari/actions/runs/10507210128/job/29108467312?pr=11672#step:8:45

… so disable it

split from #11672 https://mozilla-hub.atlassian.net/browse/MP-1456

LeoMcA · 2024-08-23T13:24:22Z

@fiji-flo as requested, benchmarking yarn render:html on main vs this branch for en-us content:

main:

$ cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node build/ssr-cli.ts
WARNING: Skipped rendering HTML. Document is missing url: /workspace/yari/client/build/en-us/search-index.json
Rendered 12,794 pages in 22.7 seconds, at a rate of 562.7 documents per second.
Done in 24.71s.
yarn render:html  27.35s user 9.38s system 147% cpu 24.856 total

this branch:

$ cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node build/ssr-cli.ts
WARNING: Skipped rendering HTML. Document is missing url: /workspace/yari/client/build/en-us/search-index.json
Rendered 12,794 pages in 22.6 seconds, at a rate of 566.9 documents per second.
Done in 24.63s.
yarn render:html  28.13s user 8.00s system 145% cpu 24.777 total

So it's almost exactly as fast.

github-actions · 2024-09-05T11:55:44Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2024-09-06T15:48:26Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2024-09-09T14:25:38Z

This pull request has merge conflicts that must be resolved before it can be merged.

caugner

LGTM overall, left some comments.

PS: To double-check, does this have any impact on localhost:3000 or (particularly) localhost:5042?

client/config/webpack.config.js

client/public/index.html

ssr/prepare.ts

ssr/theme.js

testing/tests/index.test.ts

testing/tests/csp.test.ts

ssr/webpack.config.js

ssr/.gitignore

package.json

LeoMcA · 2024-09-10T15:09:57Z

PS: To double-check, does this have any impact on localhost:3000 or (particularly) localhost:5042?

There's a change in localhost:3000, in that we're now using a very plain index.html template, rather than the one with all the dummy code in it for ssr/render to find and replace - but it still works as it did.

No change to localhost:5042, other than the changes to stage/prod as well: i.e. a few html formatting changes with self closing tags and empty attributes etc.

github-actions · 2024-09-10T17:22:55Z

This pull request has merge conflicts that must be resolved before it can be merged.

LeoMcA added 2 commits August 22, 2024 10:02

chore(ssr): rename render.ts to render.tsx

19a1854

chore(ssr): use jsx in ssr/render.tsx

96fd148

generate csp hashes in webpack remove manual asset hashing

LeoMcA requested review from mdn-bot and a team as code owners August 22, 2024 11:12

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

fix(ssr): test for asset-manifest.json in yarn start

3f27fe5

LeoMcA marked this pull request as draft August 22, 2024 11:32

LeoMcA added 8 commits August 22, 2024 14:27

fix(ssr): weird path resolution in libs/env breaking npm-published-si…

03ac9fe

…mulation https://github.com/mdn/yari/actions/runs/10507210128/job/29108467312?pr=11672#step:8:45

fix(ssr): fix failing index.html tests

84b8942

fix(ssr): handle absolute paths in libs/env

73fe2c2

fix tests

6730b61

revert changes to libs/env, use actual client webpack outputPath

80abd8b

🙏

1285538

webpack-node-externals wasn't working, but now it works things break,…

2f35d67

… so disable it

fix tests

49c5c46

LeoMcA added a commit that referenced this pull request Aug 23, 2024

chore(ssr): delete root index.html

2db5633

split from #11672 https://mozilla-hub.atlassian.net/browse/MP-1456

LeoMcA mentioned this pull request Aug 23, 2024

chore(ssr): delete root index.html #11677

Closed

github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Sep 5, 2024

Merge remote-tracking branch 'origin/main' into ssr-render-jsx

abec174

github-actions bot added merge conflicts 🚧 Please rebase onto or merge the latest main. and removed merge conflicts 🚧 Please rebase onto or merge the latest main. labels Sep 6, 2024

Merge remote-tracking branch 'origin/main' into ssr-render-jsx

bd4279c

github-actions bot added merge conflicts 🚧 Please rebase onto or merge the latest main. and removed merge conflicts 🚧 Please rebase onto or merge the latest main. labels Sep 6, 2024

Merge remote-tracking branch 'origin/main' into ssr-render-jsx

53da59c

github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Sep 10, 2024

LeoMcA marked this pull request as ready for review September 10, 2024 10:12

caugner reviewed Sep 10, 2024

View reviewed changes

LeoMcA added 2 commits September 10, 2024 15:12

Merge remote-tracking branch 'origin/main' into ssr-render-jsx

cc97a91

wip(ssr): review updates

7c2a1bc

github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Sep 10, 2024

Merge remote-tracking branch 'origin/main' into ssr-render-jsx

afaa1cc

github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Sep 16, 2024

caugner approved these changes Sep 16, 2024

View reviewed changes

caugner changed the title ~~chore(ssr): use jsx in ssr/render.tsx~~ feat(ssr): render JSX instead of index.html Sep 16, 2024

LeoMcA merged commit 6b75edd into main Sep 16, 2024
16 checks passed

LeoMcA deleted the ssr-render-jsx branch September 16, 2024 12:23

github-actions bot mentioned this pull request Sep 16, 2024

chore(main): release 2.64.0 #11813

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

LeoMcA commented Aug 22, 2024

LeoMcA commented Aug 23, 2024

github-actions bot commented Sep 5, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 9, 2024

caugner left a comment

LeoMcA commented Sep 10, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024

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

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

Conversation

LeoMcA commented Aug 22, 2024

Summary

Problem

Solution

How did you test this change?

LeoMcA commented Aug 23, 2024

github-actions bot commented Sep 5, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 9, 2024

caugner left a comment

Choose a reason for hiding this comment

LeoMcA commented Sep 10, 2024 • edited Loading

github-actions bot commented Sep 10, 2024

LeoMcA commented Sep 10, 2024 •

edited

Loading