Keep prerender component tree the same as regular render tree (#7760)

* Keep prerender component tree the same as regular render tree * Comment loader() type cast * Update test project * renderedLoadingState * syncLoader * No renderLoadingState * set state depending on prerender * Fix unprerendered pages * Use require.resolveWeak * save progress * working changes - update babel plugin to add webpack chunk name - remove server markup - revert hydrate logic - insert chunk in prerendered html file * clean up from late-night pairing session * set with prerender prop * rename syncLoader to prerenderLoader * update comment * add wip codemod * finish codemod * add codemod test * Update unit tests to match new page loaders * Update index.html in test project fixture * active-route-loader: fix initial load * Only use prerenderLoader in production * Detect prerendered pages * revert changes to analyzeRouterTree * Don't try to load separate chunk for / * createCell: Suspense null fallback
redwoodjs · Mar 27, 2023 · 0063292 · 0063292
1 parent 12b9f4f
commit 0063292
Show file tree

Hide file tree

Showing 19 changed files with 353 additions and 131 deletions.
diff --git a/__fixtures__/test-project/web/src/index.html b/__fixtures__/test-project/web/src/index.html
@@ -8,10 +8,8 @@
 </head>
 
 <body>
-  <div id="redwood-app">
-    <!-- Please keep the line below for prerender support. -->
-    <%= prerenderPlaceholder %>
-  </div>
+  <!-- Please keep this div empty -->
+  <div id="redwood-app"></div>
 </body>
 
 </html>
diff --git a/packages/codemods/package.json b/packages/codemods/package.json
@@ -31,6 +31,7 @@
     "@iarna/toml": "2.2.5",
     "@vscode/ripgrep": "1.15.0",
     "@whatwg-node/fetch": "0.8.4",
+    "cheerio": "1.0.0-rc.12",
     "core-js": "3.29.1",
     "deepmerge": "4.3.1",
     "execa": "5.1.1",

diff --git a/packages/codemods/src/codemods/v5.x.x/checkReactRoot/README.md b/packages/codemods/src/codemods/v5.x.x/checkReactRoot/README.md
@@ -0,0 +1,27 @@
+# Check React Root
+
+React 18 doesn't handle hydration errors the same way 17 did. It's very strict, so we have to be very careful about the server HTML we send to the browser to be hydrated.
+
+In v5, we changed the default index.html file a bit—we removed the `prerenderPlaceholder`:
+
+```diff
+ <!DOCTYPE html>
+ <html lang="en">
+
+ <head>
+   <meta charset="UTF-8" />
+   <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+   <link rel="icon" type="image/png" href="/favicon.png" />
+ </head>
+
+ <body>
+   <div id="redwood-app">
+-    <!-- Please keep the line below for prerender support. -->
+-    <%= prerenderPlaceholder %>
+   </div>
+ </body>
+
+ </html>
+```
+
+This codemod removes that templating syntax from a user's `index.html` file, and warns about other children in the react root.
diff --git a/...mods/src/codemods/v5.x.x/checkReactRoot/__testfixtures__/default/input/web/src/index.html b/...mods/src/codemods/v5.x.x/checkReactRoot/__testfixtures__/default/input/web/src/index.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+  <meta charset="UTF-8" />
+  <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+  <link rel="icon" type="image/png" href="/favicon.png" />
+</head>
+
+<body>
+  <div id="redwood-app">
+    <!-- Please keep the line below for prerender support. -->
+    <%= prerenderPlaceholder %>
+    <div>I shouldn't be here</div>
+  </div>
+</body>
+
+</html>
diff --git a/...ods/src/codemods/v5.x.x/checkReactRoot/__testfixtures__/default/output/web/src/index.html b/...ods/src/codemods/v5.x.x/checkReactRoot/__testfixtures__/default/output/web/src/index.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html><html lang="en"><head>
+  <meta charset="UTF-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <link rel="icon" type="image/png" href="/favicon.png">
+</head>
+
+<body>
+  <div id="redwood-app"></div>
+
+
+
+<div>I shouldn't be here</div></body></html>
diff --git a/packages/codemods/src/codemods/v5.x.x/checkReactRoot/__tests__/checkReactRoot.test.ts b/packages/codemods/src/codemods/v5.x.x/checkReactRoot/__tests__/checkReactRoot.test.ts
@@ -0,0 +1,7 @@
+import { checkReactRoot } from '../checkReactRoot'
+
+describe('tsconfigForRouteHooks', () => {
+  it('Checks the react root', async () => {
+    await matchFolderTransform(checkReactRoot, 'default')
+  })
+})
diff --git a/packages/codemods/src/codemods/v5.x.x/checkReactRoot/checkReactRoot.ts b/packages/codemods/src/codemods/v5.x.x/checkReactRoot/checkReactRoot.ts
@@ -0,0 +1,36 @@
+import fs from 'fs'
+import path from 'path'
+
+import { load } from 'cheerio'
+
+import getRWPaths from '../../../lib/getRWPaths'
+
+export function checkReactRoot() {
+  const indexHTMLFilepath = path.join(
+    getRWPaths().web.base,
+    'src',
+    'index.html'
+  )
+
+  const indexHTML = load(fs.readFileSync(indexHTMLFilepath, 'utf-8'))
+
+  const reactRoot = indexHTML('#redwood-app')
+  const reactRootChildren = reactRoot.children()
+
+  if (reactRootChildren.length) {
+    console.log(
+      [
+        `The react root (<div id="redwood-app"></div>) in ${indexHTMLFilepath} has children:`,
+        reactRoot.html(),
+        'React expects to control this DOM node completely. This codemod has moved the children outside the react root',
+        'but consider moving them into a layout.',
+        '',
+      ].join('\n')
+    )
+  }
+
+  indexHTML('body').append(reactRootChildren)
+  reactRoot.text('')
+
+  fs.writeFileSync(indexHTMLFilepath, indexHTML.html())
+}
diff --git a/packages/codemods/src/codemods/v5.x.x/checkReactRoot/checkReactRoot.yargs.ts b/packages/codemods/src/codemods/v5.x.x/checkReactRoot/checkReactRoot.yargs.ts
@@ -0,0 +1,13 @@
+import task, { TaskInnerAPI } from 'tasuku'
+
+import { checkReactRoot } from './checkReactRoot'
+
+export const command = 'check-react-root'
+export const description = '(v5.x.x->v5.x.x) Converts world to bazinga'
+
+export const handler = () => {
+  task('Check React Root', ({ setOutput }: TaskInnerAPI) => {
+    checkReactRoot()
+    setOutput('All done!')
+  })
+}
diff --git a/packages/core/config/webpack.common.js b/packages/core/config/webpack.common.js
@@ -230,11 +230,6 @@ module.exports = (webpackEnv) => {
       !isEnvProduction && new webpack.HotModuleReplacementPlugin(),
       new HtmlWebpackPlugin({
         template: path.resolve(redwoodPaths.base, 'web/src/index.html'),
-        templateParameters: {
-          prerenderPlaceholder: isEnvProduction
-            ? '<server-markup></server-markup>'
-            : '<!-- Prerender placeholder -->',
-        },
         scriptLoading: 'defer',
         inject: true,
         chunks: 'all',

diff --git a/packages/create-redwood-app/template/web/src/index.html b/packages/create-redwood-app/template/web/src/index.html
@@ -8,10 +8,8 @@
 </head>
 
 <body>
-  <div id="redwood-app">
-    <!-- Please keep the line below for prerender support. -->
-    <%= prerenderPlaceholder %>
-  </div>
+  <!-- Please keep this div empty -->
+  <div id="redwood-app"></div>
 </body>
 
 </html>
diff --git a/packages/internal/src/__tests__/nestedPages.test.ts b/packages/internal/src/__tests__/nestedPages.test.ts
@@ -50,131 +50,159 @@ describe('User specified imports, with static imports', () => {
     )
   })
 
-  it('Adds loaders for non-nested pages, that do not have an explicit import', () => {
-    // Static import for prerender
-    expect(outputWithStaticImports).toContain(
-      `const LoginPage = {
+  describe('pages without explicit import', () => {
+    describe('static prerender imports', () => {
+      it('Adds loaders for non-nested pages', () => {
+        expect(outputWithStaticImports).toContain(
+          `const LoginPage = {
   name: "LoginPage",
-  loader: () => require("./pages/LoginPage/LoginPage")
+  loader: () => import( /* webpackChunkName: "LoginPage" */"./pages/LoginPage/LoginPage"),
+  prerenderLoader: () => require("./pages/LoginPage/LoginPage")
 }`
-    )
+        )
 
-    expect(outputWithStaticImports).toContain(
-      `const HomePage = {
+        expect(outputWithStaticImports).toContain(
+          `const HomePage = {
   name: "HomePage",
-  loader: () => require("./pages/HomePage/HomePage")
+  loader: () => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"),
+  prerenderLoader: () => require("./pages/HomePage/HomePage")
 }`
-    )
-
-    // Dynamic import for build
-    expect(outputNoStaticImports).toContain(
-      `const LoginPage = {
+        )
+      })
+    })
+
+    describe('dynamic build imports', () => {
+      it('Adds loaders for non-nested pages with __webpack_require__ and require.resolveWeak in prerenderLoader to not bundle the pages', () => {
+        expect(outputNoStaticImports).toContain(
+          `const LoginPage = {
   name: "LoginPage",
-  loader: () => import("./pages/LoginPage/LoginPage")
+  loader: () => import( /* webpackChunkName: "LoginPage" */"./pages/LoginPage/LoginPage"),
+  prerenderLoader: () => __webpack_require__(require.resolveWeak("./pages/LoginPage/LoginPage"))
 }`
-    )
+        )
 
-    expect(outputNoStaticImports).toContain(
-      `const HomePage = {
+        expect(outputNoStaticImports).toContain(
+          `const HomePage = {
   name: "HomePage",
-  loader: () => import("./pages/HomePage/HomePage")
+  loader: () => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"),
+  prerenderLoader: () => __webpack_require__(require.resolveWeak("./pages/HomePage/HomePage"))
 }`
-    )
+        )
+      })
+    })
   })
 
-  it('[static imports] Loader for imported nested page uses user specified name', () => {
-    // Import statement: import JobsPage from 'src/pages/Jobs/JobsPage'
-    expect(outputWithStaticImports).toContain(
-      `const NewJobPage = {
+  describe('pages with explicit import', () => {
+    describe('static prerender imports', () => {
+      it('Uses the user specified name for nested page', () => {
+        // Import statement: import NewJobPage from 'src/pages/Jobs/NewJobPage'
+        expect(outputWithStaticImports).toContain(
+          `const NewJobPage = {
   name: "NewJobPage",
-  loader: () => require("./pages/Jobs/NewJobPage/NewJobPage")
+  loader: () => import( /* webpackChunkName: "NewJobPage" */"./pages/Jobs/NewJobPage/NewJobPage"),
+  prerenderLoader: () => require("./pages/Jobs/NewJobPage/NewJobPage")
 }`
-    )
+        )
+      })
 
-    // Import statement is this: import BazingaJobProfilePageWithFunnyName from 'src/pages/Jobs/JobProfilePage'
-    expect(outputWithStaticImports).toContain(
-      `const BazingaJobProfilePageWithFunnyName = {
+      it('Uses the user specified custom default export import name for nested page', () => {
+        // Import statement: import BazingaJobProfilePageWithFunnyName from 'src/pages/Jobs/JobProfilePage'
+        expect(outputWithStaticImports).toContain(
+          `const BazingaJobProfilePageWithFunnyName = {
   name: "BazingaJobProfilePageWithFunnyName",
-  loader: () => require("./pages/Jobs/JobProfilePage/JobProfilePage")
+  loader: () => import( /* webpackChunkName: "BazingaJobProfilePageWithFunnyName" */"./pages/Jobs/JobProfilePage/JobProfilePage"),
+  prerenderLoader: () => require("./pages/Jobs/JobProfilePage/JobProfilePage")
 }`
-    )
-
-    // Check that the explicitly imported nested pages are removed too
-    expect(outputWithStaticImports).not.toContain(
-      `var _NewJobPage = _interopRequireDefault`
-    )
-
-    expect(outputWithStaticImports).not.toContain(
-      `var _JobProfilePage = _interopRequireDefault`
-    )
-
-    // Generate react component still uses the user specified name
-    expect(outputWithStaticImports).toContain(`.createElement(_router.Route, {
+        )
+      })
+
+      it('Removes explicit imports when prerendering', () => {
+        expect(outputWithStaticImports).not.toContain(
+          `var _NewJobPage = _interopRequireDefault`
+        )
+
+        expect(outputWithStaticImports).not.toContain(
+          `var _JobProfilePage = _interopRequireDefault`
+        )
+      })
+
+      it('Keeps using the user specified name when generating React component', () => {
+        // Generate react component still uses the user specified name
+        expect(outputWithStaticImports)
+          .toContain(`.createElement(_router.Route, {
     path: "/job-profiles/{id:Int}",
     page: BazingaJobProfilePageWithFunnyName,
     name: "jobProfile"
   })`)
-  })
-
-  it('[no static import] Directly uses the import when page is explicitly imported', () => {
-    // Explicit import uses the specified import
-    // Has statement: import BazingaJobProfilePageWithFunnyName from 'src/pages/Jobs/JobProfilePage'
-    // The name of the import is not important without static imports
-    expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
+      })
+    })
+
+    describe('dynamic build imports', () => {
+      it('Directly uses the import when page is explicitly imported', () => {
+        // Explicit import uses the specified import
+        // Has statement: import BazingaJobProfilePageWithFunnyName from 'src/pages/Jobs/JobProfilePage'
+        // The name of the import is not important without static imports
+        expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
     path: "/job-profiles/{id:Int}",
     page: _JobProfilePage["default"],
     name: "jobProfile"
   })`)
+      })
 
-    // Uses the loader for a page that isn't imported
-    expect(outputNoStaticImports).toContain(`const HomePage = {
+      it("Uses the loader for a page that isn't imported", () => {
+        expect(outputNoStaticImports).toContain(`const HomePage = {
   name: "HomePage",
-  loader: () => import("./pages/HomePage/HomePage")
+  loader: () => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"),
+  prerenderLoader: () => __webpack_require__(require.resolveWeak("./pages/HomePage/HomePage"))
 }`)
-    expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
+        expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
     path: "/",
     page: HomePage,
     name: "home"
   })`)
+      })
 
-    // Should NOT add a loader for pages that have been explicitly loaded
-    expect(outputNoStaticImports).not.toContain(`const JobsJobPage = {
+      it('Should NOT add a loader for pages that have been explicitly loaded', () => {
+        expect(outputNoStaticImports).not.toContain(`const JobsJobPage = {
   name: "JobsJobPage",
-  loader: () => import("./pages/Jobs/JobsPage/JobsPage")
-}`)
+  loader: () => import( /* webpackChunkName: "JobsJobPage" */"./pages/Jobs/JobsPage/JobsPage")`)
 
-    expect(outputNoStaticImports).not.toContain(`const JobsNewJobPage = {
+        expect(outputNoStaticImports).not.toContain(`const JobsNewJobPage = {
   name: "JobsNewJobPage",
-  loader: () => import("./pages/Jobs/NewJobPage/NewJobPage")
-}`)
+  loader: () => import( /* webpackChunkName: "JobsNewJobPage" */"./pages/Jobs/NewJobPage/NewJobPage")`)
 
-    expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
+        expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
     path: "/jobs",
     page: _JobsPage["default"],
     name: "jobs"
   })`)
+      })
+    })
   })
 
   it('Handles when imports from a page include non-default imports too', () => {
     // Because we import import EditJobPage, 👉 { NonDefaultExport } from 'src/pages/Jobs/EditJobPage'
 
     expect(outputWithStaticImports).toContain('var _EditJobPage = require("')
+
     expect(outputWithStaticImports).toContain(`const EditJobPage = {
   name: "EditJobPage",
-  loader: () => require("./pages/Jobs/EditJobPage/EditJobPage")
+  loader: () => import( /* webpackChunkName: "EditJobPage" */"./pages/Jobs/EditJobPage/EditJobPage"),
+  prerenderLoader: () => require("./pages/Jobs/EditJobPage/EditJobPage")
 }`)
 
     expect(outputNoStaticImports).toContain(
       'var _EditJobPage = _interopRequireWildcard('
     )
+
     expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
     path: "/jobs/{id:Int}/edit",
     page: _EditJobPage["default"],
     name: "editJob"`)
 
     // Should not generate a loader, because page was explicitly imported
-    expect(outputNoStaticImports).not.toContain(
-      `loader: () => import("./pages/Jobs/EditJobPage/EditJobPage")`
+    expect(outputNoStaticImports).not.toMatch(
+      /loader: \(\) => import\(.*"\.\/pages\/Jobs\/EditJobPage\/EditJobPage"\)/
     )
   })
 })