From f42a91db0a40a9abfe940b2aa6d06bc70efba157 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 7 Nov 2022 11:21:00 +0100 Subject: [PATCH 1/5] Create head file when creating root layout, also cover more cases --- packages/next/lib/verifyRootLayout.ts | 76 ++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 13 deletions(-) diff --git a/packages/next/lib/verifyRootLayout.ts b/packages/next/lib/verifyRootLayout.ts index b20e69accb149c2..f08edc55b38e208 100644 --- a/packages/next/lib/verifyRootLayout.ts +++ b/packages/next/lib/verifyRootLayout.ts @@ -26,7 +26,6 @@ function getRootLayout(isTs: boolean) { }) { return ( - {children} ) @@ -37,7 +36,6 @@ function getRootLayout(isTs: boolean) { return `export default function RootLayout({ children }) { return ( - {children} ) @@ -45,6 +43,19 @@ function getRootLayout(isTs: boolean) { ` } +function getHead() { + return `export default function Head() { + return ( + <> + + + + + ) +} +` +} + export async function verifyRootLayout({ dir, appDir, @@ -63,15 +74,38 @@ export async function verifyRootLayout({ appDir, `**/layout.{${pageExtensions.join(',')}}` ) - const hasLayout = layoutFiles.length !== 0 - const normalizedPagePath = pagePath.replace(`${APP_DIR_ALIAS}/`, '') - const firstSegmentValue = normalizedPagePath.split('/')[0] - const pageRouteGroup = firstSegmentValue.startsWith('(') - ? firstSegmentValue - : undefined + const pagePathSegments = normalizedPagePath.split('/') + + // Find an available dir to place the layout file in, the layout file can't affect any other layout. + // Place the layout as close to app/ as possible. + let availableDir: string | undefined + + if (layoutFiles.length !== 0) { + // If there's no other layout file we can place the layout file in the app dir. + // However, if the page is within a route group directly under app (e.g. app/(routegroup)/page.js) + // prefer creating the root layout in that route group. + const firstSegmentValue = pagePathSegments[0] + availableDir = firstSegmentValue.startsWith('(') ? firstSegmentValue : '' + } else { + pagePathSegments.pop() // remove the page from segments + + let currentSegments: string[] = [] + for (const segment of pagePathSegments) { + currentSegments.push(segment) + // Find the dir closest to app/ where a layout can be created without affecting other layouts. + if ( + !layoutFiles.some((file) => + file.startsWith(currentSegments.join('/')) + ) + ) { + availableDir = currentSegments.join('/') + break + } + } + } - if (pageRouteGroup || !hasLayout) { + if (typeof availableDir === 'string') { const resolvedTsConfigPath = path.join(dir, tsconfigPath) const hasTsConfig = await fs.access(resolvedTsConfigPath).then( () => true, @@ -80,19 +114,35 @@ export async function verifyRootLayout({ const rootLayoutPath = path.join( appDir, - // If the page is within a route group directly under app (e.g. app/(routegroup)/page.js) - // prefer creating the root layout in that route group. Otherwise create the root layout in the app root. - pageRouteGroup ? pageRouteGroup : '', + availableDir, `layout.${hasTsConfig ? 'tsx' : 'js'}` ) await fs.writeFile(rootLayoutPath, getRootLayout(hasTsConfig)) + const headPath = path.join( + appDir, + availableDir, + `head.${hasTsConfig ? 'tsx' : 'js'}` + ) + const hasHead = await fs.access(headPath).then( + () => true, + () => false + ) + + if (!hasHead) { + await fs.writeFile(headPath, getHead()) + } + console.log( chalk.green( `\nYour page ${chalk.bold( `app/${normalizedPagePath}` )} did not have a root layout, we created ${chalk.bold( `app${rootLayoutPath.replace(appDir, '')}` - )} for you.` + )}${ + !hasHead + ? ` and ${chalk.bold(`app${headPath.replace(appDir, '')}`)}` + : '' + } for you.` ) + '\n' ) From a2fba47917a30675ad2743af238a18be25a50590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 7 Nov 2022 11:22:18 +0100 Subject: [PATCH 2/5] fix tests --- test/e2e/app-dir/create-root-layout.test.ts | 181 +++++++++++++----- .../(group)/route/first/head.js | 9 + .../(group)/route/first}/layout.js | 1 - .../(group)/route/first/page.js | 3 + .../(group)/route/second/inner/page.js | 3 + .../app-group-layout/(group)/page.js | 3 + .../app-group-layout/(group1)/path1/page.js | 3 - .../app-group-layout/(group2)/path2/page.js | 3 - .../app-dir/create-root-layout/app/head.js | 9 + .../app-dir/create-root-layout/app/layout.js | 7 + .../app/{ => route}/page.js | 0 11 files changed, 172 insertions(+), 50 deletions(-) create mode 100644 test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/head.js rename test/e2e/app-dir/create-root-layout/{app-group-layout/(group1) => app-find-available-dir/(group)/route/first}/layout.js (85%) create mode 100644 test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/page.js create mode 100644 test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/second/inner/page.js create mode 100644 test/e2e/app-dir/create-root-layout/app-group-layout/(group)/page.js delete mode 100644 test/e2e/app-dir/create-root-layout/app-group-layout/(group1)/path1/page.js delete mode 100644 test/e2e/app-dir/create-root-layout/app-group-layout/(group2)/path2/page.js create mode 100644 test/e2e/app-dir/create-root-layout/app/head.js create mode 100644 test/e2e/app-dir/create-root-layout/app/layout.js rename test/e2e/app-dir/create-root-layout/app/{ => route}/page.js (100%) diff --git a/test/e2e/app-dir/create-root-layout.test.ts b/test/e2e/app-dir/create-root-layout.test.ts index 66eb1e85e2eaa5c..87210eaa46b7428 100644 --- a/test/e2e/app-dir/create-root-layout.test.ts +++ b/test/e2e/app-dir/create-root-layout.test.ts @@ -23,8 +23,8 @@ describe('app-dir create root layout', () => { beforeAll(async () => { next = await createNext({ files: { - 'app/page.js': new FileRef( - path.join(__dirname, 'create-root-layout/app/page.js') + 'app/route/page.js': new FileRef( + path.join(__dirname, 'create-root-layout/app/route/page.js') ), 'next.config.js': new FileRef( path.join(__dirname, 'create-root-layout/next.config.js') @@ -40,27 +40,39 @@ describe('app-dir create root layout', () => { it('create root layout', async () => { const outputIndex = next.cliOutput.length - const browser = await webdriver(next.url, '/') + const browser = await webdriver(next.url, '/route') expect(await browser.elementById('page-text').text()).toBe( 'Hello world!' ) expect(next.cliOutput.slice(outputIndex)).toInclude( - 'Your page app/page.js did not have a root layout, we created app/layout.js for you.' + 'Your page app/route/page.js did not have a root layout, we created app/layout.js and app/head.js for you.' ) expect(await next.readFile('app/layout.js')).toMatchInlineSnapshot(` - "export default function RootLayout({ children }) { - return ( - - - {children} - - ) - } - " - `) + "export default function RootLayout({ children }) { + return ( + + {children} + + ) + } + " + `) + + expect(await next.readFile('app/head.js')).toMatchInlineSnapshot(` + "export default function Head() { + return ( + <> + + + + + ) + } + " + `) }) }) @@ -85,28 +97,99 @@ describe('app-dir create root layout', () => { it('create root layout', async () => { const outputIndex = next.cliOutput.length - const browser = await webdriver(next.url, '/path2') + const browser = await webdriver(next.url, '/') expect(await browser.elementById('page-text').text()).toBe( - 'Hello world 2' + 'Hello world' ) expect(next.cliOutput.slice(outputIndex)).toInclude( - 'Your page app/(group2)/path2/page.js did not have a root layout, we created app/(group2)/layout.js for you.' + 'Your page app/(group)/page.js did not have a root layout, we created app/(group)/layout.js and app/(group)/head.js for you.' ) - expect(await next.readFile('app/(group2)/layout.js')) + expect(await next.readFile('app/(group)/layout.js')) + .toMatchInlineSnapshot(` + "export default function RootLayout({ children }) { + return ( + + {children} + + ) + } + " + `) + + expect(await next.readFile('app/(group)/layout.js')) .toMatchInlineSnapshot(` - "export default function RootLayout({ children }) { - return ( - - - {children} - + "export default function RootLayout({ children }) { + return ( + + {children} + + ) + } + " + `) + }) + }) + + describe('find available dir', () => { + beforeAll(async () => { + next = await createNext({ + files: { + app: new FileRef( + path.join( + __dirname, + 'create-root-layout/app-find-available-dir' + ) + ), + 'next.config.js': new FileRef( + path.join(__dirname, 'create-root-layout/next.config.js') + ), + }, + dependencies: { + react: 'experimental', + 'react-dom': 'experimental', + }, + }) + }) + afterAll(() => next.destroy()) + + it('create root layout', async () => { + const outputIndex = next.cliOutput.length + const browser = await webdriver(next.url, '/route/second/inner') + + expect(await browser.elementById('page-text').text()).toBe( + 'Hello world' ) - } - " - `) + + expect(next.cliOutput.slice(outputIndex)).toInclude( + 'Your page app/(group)/route/second/inner/page.js did not have a root layout, we created app/(group)/route/second/layout.js and app/(group)/route/second/head.js for you.' + ) + + expect(await next.readFile('app/(group)/route/second/layout.js')) + .toMatchInlineSnapshot(` + "export default function RootLayout({ children }) { + return ( + + {children} + + ) + } + " + `) + + expect(await next.readFile('app/(group)/route/second/layout.js')) + .toMatchInlineSnapshot(` + "export default function RootLayout({ children }) { + return ( + + {children} + + ) + } + " + `) }) }) }) @@ -116,7 +199,7 @@ describe('app-dir create root layout', () => { next = await createNext({ files: { 'app/page.tsx': new FileRef( - path.join(__dirname, 'create-root-layout/app/page.js') + path.join(__dirname, 'create-root-layout/app/route/page.js') ), 'next.config.js': new FileRef( path.join(__dirname, 'create-root-layout/next.config.js') @@ -142,24 +225,36 @@ describe('app-dir create root layout', () => { ) expect(next.cliOutput.slice(outputIndex)).toInclude( - 'Your page app/page.tsx did not have a root layout, we created app/layout.tsx for you.' + 'Your page app/page.tsx did not have a root layout, we created app/layout.tsx and app/head.tsx for you.' ) expect(await next.readFile('app/layout.tsx')).toMatchInlineSnapshot(` - "export default function RootLayout({ - children, - }: { - children: React.ReactNode - }) { - return ( - - - {children} - - ) - } - " - `) + "export default function RootLayout({ + children, + }: { + children: React.ReactNode + }) { + return ( + + {children} + + ) + } + " + `) + + expect(await next.readFile('app/head.tsx')).toMatchInlineSnapshot(` + "export default function Head() { + return ( + <> + + + + + ) + } + " + `) }) }) } else { diff --git a/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/head.js b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/head.js new file mode 100644 index 000000000000000..772e640e75cefac --- /dev/null +++ b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/head.js @@ -0,0 +1,9 @@ +export default function Head() { + return ( + <> + + + + + ) +} diff --git a/test/e2e/app-dir/create-root-layout/app-group-layout/(group1)/layout.js b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/layout.js similarity index 85% rename from test/e2e/app-dir/create-root-layout/app-group-layout/(group1)/layout.js rename to test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/layout.js index 747270b45987a2b..803f17d863c8ad8 100644 --- a/test/e2e/app-dir/create-root-layout/app-group-layout/(group1)/layout.js +++ b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/layout.js @@ -1,7 +1,6 @@ export default function RootLayout({ children }) { return ( - {children} ) diff --git a/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/page.js b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/page.js new file mode 100644 index 000000000000000..faaea962365bb7e --- /dev/null +++ b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/first/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Hello world

+} diff --git a/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/second/inner/page.js b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/second/inner/page.js new file mode 100644 index 000000000000000..faaea962365bb7e --- /dev/null +++ b/test/e2e/app-dir/create-root-layout/app-find-available-dir/(group)/route/second/inner/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Hello world

+} diff --git a/test/e2e/app-dir/create-root-layout/app-group-layout/(group)/page.js b/test/e2e/app-dir/create-root-layout/app-group-layout/(group)/page.js new file mode 100644 index 000000000000000..faaea962365bb7e --- /dev/null +++ b/test/e2e/app-dir/create-root-layout/app-group-layout/(group)/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Hello world

+} diff --git a/test/e2e/app-dir/create-root-layout/app-group-layout/(group1)/path1/page.js b/test/e2e/app-dir/create-root-layout/app-group-layout/(group1)/path1/page.js deleted file mode 100644 index 2e60ca9ad638683..000000000000000 --- a/test/e2e/app-dir/create-root-layout/app-group-layout/(group1)/path1/page.js +++ /dev/null @@ -1,3 +0,0 @@ -export default function Page() { - return

Hello world 1

-} diff --git a/test/e2e/app-dir/create-root-layout/app-group-layout/(group2)/path2/page.js b/test/e2e/app-dir/create-root-layout/app-group-layout/(group2)/path2/page.js deleted file mode 100644 index 4a8d52f9706a252..000000000000000 --- a/test/e2e/app-dir/create-root-layout/app-group-layout/(group2)/path2/page.js +++ /dev/null @@ -1,3 +0,0 @@ -export default function Page() { - return

Hello world 2

-} diff --git a/test/e2e/app-dir/create-root-layout/app/head.js b/test/e2e/app-dir/create-root-layout/app/head.js new file mode 100644 index 000000000000000..772e640e75cefac --- /dev/null +++ b/test/e2e/app-dir/create-root-layout/app/head.js @@ -0,0 +1,9 @@ +export default function Head() { + return ( + <> + + + + + ) +} diff --git a/test/e2e/app-dir/create-root-layout/app/layout.js b/test/e2e/app-dir/create-root-layout/app/layout.js new file mode 100644 index 000000000000000..803f17d863c8ad8 --- /dev/null +++ b/test/e2e/app-dir/create-root-layout/app/layout.js @@ -0,0 +1,7 @@ +export default function RootLayout({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/create-root-layout/app/page.js b/test/e2e/app-dir/create-root-layout/app/route/page.js similarity index 100% rename from test/e2e/app-dir/create-root-layout/app/page.js rename to test/e2e/app-dir/create-root-layout/app/route/page.js From 601792ecec0cf6d2df07943ce182ebe602ceed2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 7 Nov 2022 12:11:36 +0100 Subject: [PATCH 3/5] Fix --- packages/next/lib/verifyRootLayout.ts | 2 +- test/e2e/app-dir/create-root-layout.test.ts | 23 +++++++++++++++---- .../app-dir/create-root-layout/app/head.js | 9 -------- .../app-dir/create-root-layout/app/layout.js | 7 ------ 4 files changed, 20 insertions(+), 21 deletions(-) delete mode 100644 test/e2e/app-dir/create-root-layout/app/head.js delete mode 100644 test/e2e/app-dir/create-root-layout/app/layout.js diff --git a/packages/next/lib/verifyRootLayout.ts b/packages/next/lib/verifyRootLayout.ts index f08edc55b38e208..941170335265ab0 100644 --- a/packages/next/lib/verifyRootLayout.ts +++ b/packages/next/lib/verifyRootLayout.ts @@ -81,7 +81,7 @@ export async function verifyRootLayout({ // Place the layout as close to app/ as possible. let availableDir: string | undefined - if (layoutFiles.length !== 0) { + if (layoutFiles.length === 0) { // If there's no other layout file we can place the layout file in the app dir. // However, if the page is within a route group directly under app (e.g. app/(routegroup)/page.js) // prefer creating the root layout in that route group. diff --git a/test/e2e/app-dir/create-root-layout.test.ts b/test/e2e/app-dir/create-root-layout.test.ts index 87210eaa46b7428..6dc85575e8ce7c2 100644 --- a/test/e2e/app-dir/create-root-layout.test.ts +++ b/test/e2e/app-dir/create-root-layout.test.ts @@ -2,6 +2,7 @@ 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 } from 'next-test-utils' describe('app-dir create root layout', () => { const isDev = (global as any).isNextDev @@ -23,9 +24,7 @@ describe('app-dir create root layout', () => { beforeAll(async () => { next = await createNext({ files: { - 'app/route/page.js': new FileRef( - path.join(__dirname, 'create-root-layout/app/route/page.js') - ), + app: new FileRef(path.join(__dirname, 'create-root-layout/app')), 'next.config.js': new FileRef( path.join(__dirname, 'create-root-layout/next.config.js') ), @@ -46,7 +45,11 @@ describe('app-dir create root layout', () => { 'Hello world!' ) - expect(next.cliOutput.slice(outputIndex)).toInclude( + await check( + () => next.cliOutput.slice(outputIndex), + /did not have a root layout/ + ) + expect(next.cliOutput.slice(outputIndex)).toMatch( 'Your page app/route/page.js did not have a root layout, we created app/layout.js and app/head.js for you.' ) @@ -103,6 +106,10 @@ describe('app-dir create root layout', () => { 'Hello world' ) + await check( + () => next.cliOutput.slice(outputIndex), + /did not have a root layout/ + ) expect(next.cliOutput.slice(outputIndex)).toInclude( 'Your page app/(group)/page.js did not have a root layout, we created app/(group)/layout.js and app/(group)/head.js for you.' ) @@ -163,6 +170,10 @@ describe('app-dir create root layout', () => { 'Hello world' ) + await check( + () => next.cliOutput.slice(outputIndex), + /did not have a root layout/ + ) expect(next.cliOutput.slice(outputIndex)).toInclude( 'Your page app/(group)/route/second/inner/page.js did not have a root layout, we created app/(group)/route/second/layout.js and app/(group)/route/second/head.js for you.' ) @@ -224,6 +235,10 @@ describe('app-dir create root layout', () => { 'Hello world!' ) + await check( + () => next.cliOutput.slice(outputIndex), + /did not have a root layout/ + ) expect(next.cliOutput.slice(outputIndex)).toInclude( 'Your page app/page.tsx did not have a root layout, we created app/layout.tsx and app/head.tsx for you.' ) diff --git a/test/e2e/app-dir/create-root-layout/app/head.js b/test/e2e/app-dir/create-root-layout/app/head.js deleted file mode 100644 index 772e640e75cefac..000000000000000 --- a/test/e2e/app-dir/create-root-layout/app/head.js +++ /dev/null @@ -1,9 +0,0 @@ -export default function Head() { - return ( - <> - - - - - ) -} diff --git a/test/e2e/app-dir/create-root-layout/app/layout.js b/test/e2e/app-dir/create-root-layout/app/layout.js deleted file mode 100644 index 803f17d863c8ad8..000000000000000 --- a/test/e2e/app-dir/create-root-layout/app/layout.js +++ /dev/null @@ -1,7 +0,0 @@ -export default function RootLayout({ children }) { - return ( - - {children} - - ) -} From 702d500546072f1eae8183bd8f62edeb689940ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 7 Nov 2022 12:55:47 +0100 Subject: [PATCH 4/5] Fix test --- test/e2e/app-dir/create-root-layout.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/app-dir/create-root-layout.test.ts b/test/e2e/app-dir/create-root-layout.test.ts index 6dc85575e8ce7c2..8cfaa8a75545142 100644 --- a/test/e2e/app-dir/create-root-layout.test.ts +++ b/test/e2e/app-dir/create-root-layout.test.ts @@ -279,7 +279,7 @@ describe('app-dir create root layout', () => { skipStart: true, files: { 'app/page.js': new FileRef( - path.join(__dirname, 'create-root-layout/app/page.js') + path.join(__dirname, 'create-root-layout/app/route/page.js') ), 'next.config.js': new FileRef( path.join(__dirname, 'create-root-layout/next.config.js') From 2b738c45571042ed831f5671667d5b3915442313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Tue, 8 Nov 2022 09:38:21 +0100 Subject: [PATCH 5/5] Add empty when creating root layout --- packages/next/lib/verifyRootLayout.ts | 2 ++ test/e2e/app-dir/create-root-layout.test.ts | 28 +++++++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/next/lib/verifyRootLayout.ts b/packages/next/lib/verifyRootLayout.ts index 941170335265ab0..144184b784f6541 100644 --- a/packages/next/lib/verifyRootLayout.ts +++ b/packages/next/lib/verifyRootLayout.ts @@ -26,6 +26,7 @@ function getRootLayout(isTs: boolean) { }) { return ( + {children} ) @@ -36,6 +37,7 @@ function getRootLayout(isTs: boolean) { return `export default function RootLayout({ children }) { return ( + {children} ) diff --git a/test/e2e/app-dir/create-root-layout.test.ts b/test/e2e/app-dir/create-root-layout.test.ts index 8cfaa8a75545142..da4eeab95e6404c 100644 --- a/test/e2e/app-dir/create-root-layout.test.ts +++ b/test/e2e/app-dir/create-root-layout.test.ts @@ -57,6 +57,7 @@ describe('app-dir create root layout', () => { "export default function RootLayout({ children }) { return ( + {children} ) @@ -119,6 +120,7 @@ describe('app-dir create root layout', () => { "export default function RootLayout({ children }) { return ( + {children} ) @@ -126,13 +128,15 @@ describe('app-dir create root layout', () => { " `) - expect(await next.readFile('app/(group)/layout.js')) + expect(await next.readFile('app/(group)/head.js')) .toMatchInlineSnapshot(` - "export default function RootLayout({ children }) { + "export default function Head() { return ( - - {children} - + <> + + + + ) } " @@ -183,6 +187,7 @@ describe('app-dir create root layout', () => { "export default function RootLayout({ children }) { return ( + {children} ) @@ -190,13 +195,15 @@ describe('app-dir create root layout', () => { " `) - expect(await next.readFile('app/(group)/route/second/layout.js')) + expect(await next.readFile('app/(group)/route/second/head.js')) .toMatchInlineSnapshot(` - "export default function RootLayout({ children }) { + "export default function Head() { return ( - - {children} - + <> + + + + ) } " @@ -251,6 +258,7 @@ describe('app-dir create root layout', () => { }) { return ( + {children} )