From 6b88b492c4f3b79871152adb4c849f43b24e12ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 3 Oct 2022 21:27:16 +0200 Subject: [PATCH] Check required root layout tags (#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) --- packages/next/server/app-render.tsx | 3 ++ .../next/server/node-web-streams-helper.ts | 43 +++++++++++++++++++ test/e2e/app-dir/app-alias/app/layout.tsx | 8 ++++ ...navigation.test.ts => root-layout.test.ts} | 33 ++++++++++++-- .../nested-route-group/page.js | 0 .../(mpa-navigation)}/(route-group)/layout.js | 0 .../(route-group)/route-group/page.js | 0 .../basic-route/inner/page.js | 0 .../(mpa-navigation)}/basic-route/layout.js | 0 .../app/(mpa-navigation)}/basic-route/page.js | 0 .../dynamic/first/[param]/page.js | 0 .../app/(mpa-navigation)}/dynamic/layout.js | 0 .../dynamic/second/[param]/page.js | 0 .../(mpa-navigation)}/to-pages-dir/layout.js | 0 .../(mpa-navigation)}/to-pages-dir/page.js | 0 .../with-parallel-routes/@one/inner/page.js | 0 .../with-parallel-routes/@one/page.js | 0 .../with-parallel-routes/@two/page.js | 0 .../with-parallel-routes/layout.js | 0 .../app/(required-tags)/has-tags/layout.js | 10 +++++ .../app/(required-tags)/has-tags/page.js | 5 +++ .../(required-tags)/missing-tags/layout.js | 3 ++ .../app/(required-tags)/missing-tags/page.js | 3 ++ .../next.config.js | 0 24 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 test/e2e/app-dir/app-alias/app/layout.tsx rename test/e2e/app-dir/{mpa-navigation.test.ts => root-layout.test.ts} (80%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/(route-group)/(nested-route-group)/nested-route-group/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/(route-group)/layout.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/(route-group)/route-group/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/basic-route/inner/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/basic-route/layout.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/basic-route/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/dynamic/first/[param]/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/dynamic/layout.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/dynamic/second/[param]/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/to-pages-dir/layout.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/to-pages-dir/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/with-parallel-routes/@one/inner/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/with-parallel-routes/@one/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/with-parallel-routes/@two/page.js (100%) rename test/e2e/app-dir/{mpa-navigation/app => root-layout/app/(mpa-navigation)}/with-parallel-routes/layout.js (100%) create mode 100644 test/e2e/app-dir/root-layout/app/(required-tags)/has-tags/layout.js create mode 100644 test/e2e/app-dir/root-layout/app/(required-tags)/has-tags/page.js create mode 100644 test/e2e/app-dir/root-layout/app/(required-tags)/missing-tags/layout.js create mode 100644 test/e2e/app-dir/root-layout/app/(required-tags)/missing-tags/page.js rename test/e2e/app-dir/{mpa-navigation => root-layout}/next.config.js (100%) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index ae57f0fb60e15..98421fb1fed11 100644 --- 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 index a3ec622baa19d..943a2c29e848b 100644 --- a/packages/next/server/node-web-streams-helper.ts +++ b/packages/next/server/node-web-streams-helper.ts @@ -257,9 +257,50 @@ 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(' 0) { + controller.error( + 'Missing required root layout tags: ' + missingTags.join(', ') + ) + } + }, + }) +} + export async function continueFromInitialStream( renderStream: ReactReadableStream, { + dev, suffix, dataStream, generateStaticHTML, @@ -267,6 +308,7 @@ export async function continueFromInitialStream( serverInsertedHTMLToHead, polyfills, }: { + dev?: boolean suffix?: string dataStream?: ReadableStream 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 new file mode 100644 index 0000000000000..079c59e3e2581 --- /dev/null +++ b/test/e2e/app-dir/app-alias/app/layout.tsx @@ -0,0 +1,8 @@ +export default function Root({ children }: { children: React.ReactNode }) { + return ( + + + {children} + + ) +} diff --git a/test/e2e/app-dir/mpa-navigation.test.ts b/test/e2e/app-dir/root-layout.test.ts similarity index 80% rename from test/e2e/app-dir/mpa-navigation.test.ts rename to test/e2e/app-dir/root-layout.test.ts index 2271d2a1a2b9f..6c74b8f4f020e 100644 --- a/test/e2e/app-dir/mpa-navigation.test.ts +++ b/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/test/e2e/app-dir/mpa-navigation/app/(route-group)/(nested-route-group)/nested-route-group/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/(route-group)/(nested-route-group)/nested-route-group/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/(route-group)/(nested-route-group)/nested-route-group/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/(route-group)/(nested-route-group)/nested-route-group/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/(route-group)/layout.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/(route-group)/layout.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/(route-group)/layout.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/(route-group)/layout.js diff --git a/test/e2e/app-dir/mpa-navigation/app/(route-group)/route-group/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/(route-group)/route-group/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/(route-group)/route-group/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/(route-group)/route-group/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/basic-route/inner/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/basic-route/inner/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/basic-route/inner/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/basic-route/inner/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/basic-route/layout.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/basic-route/layout.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/basic-route/layout.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/basic-route/layout.js diff --git a/test/e2e/app-dir/mpa-navigation/app/basic-route/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/basic-route/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/basic-route/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/basic-route/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/dynamic/first/[param]/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic/first/[param]/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/dynamic/first/[param]/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic/first/[param]/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/dynamic/layout.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic/layout.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/dynamic/layout.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic/layout.js diff --git a/test/e2e/app-dir/mpa-navigation/app/dynamic/second/[param]/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic/second/[param]/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/dynamic/second/[param]/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic/second/[param]/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/to-pages-dir/layout.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/to-pages-dir/layout.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/to-pages-dir/layout.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/to-pages-dir/layout.js diff --git a/test/e2e/app-dir/mpa-navigation/app/to-pages-dir/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/to-pages-dir/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/to-pages-dir/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/to-pages-dir/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/@one/inner/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/with-parallel-routes/@one/inner/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/@one/inner/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/with-parallel-routes/@one/inner/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/@one/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/with-parallel-routes/@one/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/@one/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/with-parallel-routes/@one/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/@two/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/with-parallel-routes/@two/page.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/@two/page.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/with-parallel-routes/@two/page.js diff --git a/test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/layout.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/with-parallel-routes/layout.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/app/with-parallel-routes/layout.js rename to test/e2e/app-dir/root-layout/app/(mpa-navigation)/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 new file mode 100644 index 0000000000000..c84b681925ebc --- /dev/null +++ b/test/e2e/app-dir/root-layout/app/(required-tags)/has-tags/layout.js @@ -0,0 +1,10 @@ +export default function Root({ children }) { + return ( + + + Hello World + + {children} + + ) +} 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 new file mode 100644 index 0000000000000..8cec6435a30fb --- /dev/null +++ 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 To incorrect root layout +} 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 new file mode 100644 index 0000000000000..1b6c561ff3244 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..d28e83c6ecd27 --- /dev/null +++ b/test/e2e/app-dir/root-layout/app/(required-tags)/missing-tags/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

WORLD!

+} diff --git a/test/e2e/app-dir/mpa-navigation/next.config.js b/test/e2e/app-dir/root-layout/next.config.js similarity index 100% rename from test/e2e/app-dir/mpa-navigation/next.config.js rename to test/e2e/app-dir/root-layout/next.config.js