Check required root layout tags (vercel#41120)

Inspects the stream of rendered HTML in dev to make sure all required tags are rendered. Since navigating to a new root layout causes a full page navigation we don't have to worry about flight requests. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
Kikobeats · Oct 24, 2022 · 6b88b49 · 6b88b49
1 parent 10f8a37
commit 6b88b49
Show file tree

Hide file tree

Showing 24 changed files with 105 additions and 3 deletions.
diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx
@@ -685,6 +685,7 @@ export async function renderToHTMLOrFlight(
     serverCSSManifest = {},
     supportsDynamicHTML,
     ComponentMod,
+    dev,
   } = renderOpts
 
   patchFetch(ComponentMod)
@@ -1374,6 +1375,7 @@ export async function renderToHTMLOrFlight(
           getServerInsertedHTML,
           serverInsertedHTMLToHead: true,
           polyfills,
+          dev,
         })
       } catch (err: any) {
         // TODO-APP: show error overlay in development. `element` should probably be wrapped in AppRouter for this case.
@@ -1405,6 +1407,7 @@ export async function renderToHTMLOrFlight(
           getServerInsertedHTML,
           serverInsertedHTMLToHead: true,
           polyfills,
+          dev,
         })
       }
     }

diff --git a/packages/next/server/node-web-streams-helper.ts b/packages/next/server/node-web-streams-helper.ts
@@ -257,16 +257,58 @@ export function createSuffixStream(
   })
 }
 
+export function createRootLayoutValidatorStream(): TransformStream<
+  Uint8Array,
+  Uint8Array
+> {
+  let foundHtml = false
+  let foundHead = false
+  let foundBody = false
+
+  return new TransformStream({
+    async transform(chunk, controller) {
+      if (!foundHtml || !foundHead || !foundBody) {
+        const content = decodeText(chunk)
+        if (!foundHtml && content.includes('<html')) {
+          foundHtml = true
+        }
+        if (!foundHead && content.includes('<head')) {
+          foundHead = true
+        }
+        if (!foundBody && content.includes('<body')) {
+          foundBody = true
+        }
+      }
+      controller.enqueue(chunk)
+    },
+    flush(controller) {
+      const missingTags = [
+        foundHtml ? null : 'html',
+        foundHead ? null : 'head',
+        foundBody ? null : 'body',
+      ].filter(nonNullable)
+
+      if (missingTags.length > 0) {
+        controller.error(
+          'Missing required root layout tags: ' + missingTags.join(', ')
+        )
+      }
+    },
+  })
+}
+
 export async function continueFromInitialStream(
   renderStream: ReactReadableStream,
   {
+    dev,
     suffix,
     dataStream,
     generateStaticHTML,
     getServerInsertedHTML,
     serverInsertedHTMLToHead,
     polyfills,
   }: {
+    dev?: boolean
     suffix?: string
     dataStream?: ReadableStream<Uint8Array>
     generateStaticHTML: boolean
@@ -312,6 +354,7 @@ export async function continueFromInitialStream(
           : ''
       return polyfillScripts + serverInsertedHTML
     }),
+    dev ? createRootLayoutValidatorStream() : null,
   ].filter(nonNullable)
 
   return transforms.reduce(

diff --git a/test/e2e/app-dir/app-alias/app/layout.tsx b/test/e2e/app-dir/app-alias/app/layout.tsx
@@ -0,0 +1,8 @@
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <head></head>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/mpa-navigation.test.ts → test/e2e/app-dir/root-layout.test.ts b/test/e2e/app-dir/mpa-navigation.test.ts → test/e2e/app-dir/root-layout.test.ts
@@ -2,8 +2,11 @@ import path from 'path'
 import { createNext, FileRef } from 'e2e-utils'
 import { NextInstance } from 'test/lib/next-modes/base'
 import webdriver from 'next-webdriver'
+import { check, renderViaHTTP } from 'next-test-utils'
+
+describe('app-dir root layout', () => {
+  const isDev = (global as any).isNextDev
 
-describe('app-dir mpa navigation', () => {
   if ((global as any).isNextDeploy) {
     it('should skip next deploy for now', () => {})
     return
@@ -18,9 +21,9 @@ describe('app-dir mpa navigation', () => {
   beforeAll(async () => {
     next = await createNext({
       files: {
-        app: new FileRef(path.join(__dirname, 'mpa-navigation/app')),
+        app: new FileRef(path.join(__dirname, 'root-layout/app')),
         'next.config.js': new FileRef(
-          path.join(__dirname, 'mpa-navigation/next.config.js')
+          path.join(__dirname, 'root-layout/next.config.js')
         ),
       },
       dependencies: {
@@ -31,6 +34,30 @@ describe('app-dir mpa navigation', () => {
   })
   afterAll(() => next.destroy())
 
+  if (isDev) {
+    describe('Missing required tags', () => {
+      it('should error on page load', async () => {
+        const outputIndex = next.cliOutput.length
+        renderViaHTTP(next.url, '/missing-tags').catch(() => {})
+        await check(
+          () => next.cliOutput.slice(outputIndex),
+          /Missing required root layout tags: html, head, body/
+        )
+      })
+
+      it('should error on page navigation', async () => {
+        const outputIndex = next.cliOutput.length
+        const browser = await webdriver(next.url, '/has-tags')
+        await browser.elementByCss('a').click()
+
+        await check(
+          () => next.cliOutput.slice(outputIndex),
+          /Missing required root layout tags: html, head, body/
+        )
+      })
+    })
+  }
+
   describe('Should do a mpa navigation when switching root layout', () => {
     it('should work with basic routes', async () => {
       const browser = await webdriver(next.url, '/basic-route')

diff --git a/...d-route-group)/nested-route-group/page.js → ...d-route-group)/nested-route-group/page.js b/...d-route-group)/nested-route-group/page.js → ...d-route-group)/nested-route-group/page.js
diff --git a/...pa-navigation/app/(route-group)/layout.js → .../(mpa-navigation)/(route-group)/layout.js b/...pa-navigation/app/(route-group)/layout.js → .../(mpa-navigation)/(route-group)/layout.js
diff --git a/...ion/app/(route-group)/route-group/page.js → ...gation)/(route-group)/route-group/page.js b/...ion/app/(route-group)/route-group/page.js → ...gation)/(route-group)/route-group/page.js
diff --git a/...-navigation/app/basic-route/inner/page.js → ...mpa-navigation)/basic-route/inner/page.js b/...-navigation/app/basic-route/inner/page.js → ...mpa-navigation)/basic-route/inner/page.js
diff --git a/.../mpa-navigation/app/basic-route/layout.js → ...pp/(mpa-navigation)/basic-route/layout.js b/.../mpa-navigation/app/basic-route/layout.js → ...pp/(mpa-navigation)/basic-route/layout.js
diff --git a/...ir/mpa-navigation/app/basic-route/page.js → .../app/(mpa-navigation)/basic-route/page.js b/...ir/mpa-navigation/app/basic-route/page.js → .../app/(mpa-navigation)/basic-route/page.js
diff --git a/...igation/app/dynamic/first/[param]/page.js → ...navigation)/dynamic/first/[param]/page.js b/...igation/app/dynamic/first/[param]/page.js → ...navigation)/dynamic/first/[param]/page.js
diff --git a/...-dir/mpa-navigation/app/dynamic/layout.js → ...ut/app/(mpa-navigation)/dynamic/layout.js b/...-dir/mpa-navigation/app/dynamic/layout.js → ...ut/app/(mpa-navigation)/dynamic/layout.js
diff --git a/...gation/app/dynamic/second/[param]/page.js → ...avigation)/dynamic/second/[param]/page.js b/...gation/app/dynamic/second/[param]/page.js → ...avigation)/dynamic/second/[param]/page.js
diff --git a/...mpa-navigation/app/to-pages-dir/layout.js → ...p/(mpa-navigation)/to-pages-dir/layout.js b/...mpa-navigation/app/to-pages-dir/layout.js → ...p/(mpa-navigation)/to-pages-dir/layout.js
diff --git a/...r/mpa-navigation/app/to-pages-dir/page.js → ...app/(mpa-navigation)/to-pages-dir/page.js b/...r/mpa-navigation/app/to-pages-dir/page.js → ...app/(mpa-navigation)/to-pages-dir/page.js
diff --git a/...p/with-parallel-routes/@one/inner/page.js → ...)/with-parallel-routes/@one/inner/page.js b/...p/with-parallel-routes/@one/inner/page.js → ...)/with-parallel-routes/@one/inner/page.js
diff --git a/...ion/app/with-parallel-routes/@one/page.js → ...gation)/with-parallel-routes/@one/page.js b/...ion/app/with-parallel-routes/@one/page.js → ...gation)/with-parallel-routes/@one/page.js
diff --git a/...ion/app/with-parallel-routes/@two/page.js → ...gation)/with-parallel-routes/@two/page.js b/...ion/app/with-parallel-routes/@two/page.js → ...gation)/with-parallel-routes/@two/page.js
diff --git a/...gation/app/with-parallel-routes/layout.js → ...avigation)/with-parallel-routes/layout.js b/...gation/app/with-parallel-routes/layout.js → ...avigation)/with-parallel-routes/layout.js
diff --git a/test/e2e/app-dir/root-layout/app/(required-tags)/has-tags/layout.js b/test/e2e/app-dir/root-layout/app/(required-tags)/has-tags/layout.js
@@ -0,0 +1,10 @@
+export default function Root({ children }) {
+  return (
+    <html>
+      <head>
+        <title>Hello World</title>
+      </head>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/root-layout/app/(required-tags)/has-tags/page.js b/test/e2e/app-dir/root-layout/app/(required-tags)/has-tags/page.js
@@ -0,0 +1,5 @@
+import Link from 'next/link'
+
+export default function Page() {
+  return <Link href="/missing-tags">To incorrect root layout</Link>
+}
diff --git a/test/e2e/app-dir/root-layout/app/(required-tags)/missing-tags/layout.js b/test/e2e/app-dir/root-layout/app/(required-tags)/missing-tags/layout.js
@@ -0,0 +1,3 @@
+export default function Root({ children }) {
+  return children
+}
diff --git a/test/e2e/app-dir/root-layout/app/(required-tags)/missing-tags/page.js b/test/e2e/app-dir/root-layout/app/(required-tags)/missing-tags/page.js
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>WORLD!</p>
+}
diff --git a/...e2e/app-dir/mpa-navigation/next.config.js → test/e2e/app-dir/root-layout/next.config.js b/...e2e/app-dir/mpa-navigation/next.config.js → test/e2e/app-dir/root-layout/next.config.js