Move default root layout loading to view-loader

vercel · May 9, 2022 · ca170fb · ca170fb
1 parent 1ba6d9d
commit ca170fb
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 28 deletions.
diff --git a/packages/next/build/webpack/loaders/next-view-loader.ts b/packages/next/build/webpack/loaders/next-view-loader.ts
@@ -22,24 +22,34 @@ async function resolveLayoutPathsByPage({
 }) {
   const layoutPaths = new Map<string, string | undefined>()
   const parts = pagePath.split('/')
-  // if a new root is being created we shouldn't include `views/layout.js`
-  const shouldIncludeRootLayout = !parts.some(
-    (part) => part.startsWith('(') && part.endsWith(')')
-  )
-
-  for (let i = 1; i < parts.length; i++) {
-    if (i === 1 && !shouldIncludeRootLayout) continue
+  const isNewRootLayout =
+    parts[1]?.length > 2 && parts[1]?.startsWith('(') && parts[1]?.endsWith(')')
 
+  for (let i = parts.length; i >= 0; i--) {
     const pathWithoutSlashLayout = parts.slice(0, i).join('/')
-    const layoutPath = `${pathWithoutSlashLayout}/layout`
-
-    const resolvedLayoutPath = await resolve(layoutPath)
 
+    if (!pathWithoutSlashLayout) {
+      continue
+    }
+    const layoutPath = `${pathWithoutSlashLayout}/layout`
+    let resolvedLayoutPath = await resolve(layoutPath)
     let urlPath = pathToUrlPath(pathWithoutSlashLayout)
 
+    // if we are in a new root views/(root) and a custom root layout was
+    // not provided or a root layout views/layout is not present, we use
+    // a default root layout to provide the html/body tags
+    const isCustomRootLayout = isNewRootLayout && i === 2
+
+    if ((isCustomRootLayout || i === 1) && !resolvedLayoutPath) {
+      resolvedLayoutPath = await resolve('next/dist/lib/views-layout')
+    }
     layoutPaths.set(urlPath, resolvedLayoutPath)
-  }
 
+    // if we're in a new root layout don't add the top-level view/layout
+    if (isCustomRootLayout) {
+      break
+    }
+  }
   return layoutPaths
 }
 

diff --git a/packages/next/server/view-render.tsx b/packages/next/server/view-render.tsx
@@ -19,7 +19,6 @@ import {
 import { FlushEffectsContext } from '../shared/lib/flush-effects'
 import { isDynamicRoute } from '../shared/lib/router/utils'
 import { tryGetPreviewData } from './api-utils/node'
-import DefaultRootLayout from '../lib/views-layout'
 
 const ReactDOMServer = process.env.__NEXT_REACT_ROOT
   ? require('react-dom/server.browser')
@@ -241,20 +240,8 @@ export async function renderToHTML(
       return mod
     })
 
-  // we need to add the default root layout when
-  // not rendering a sub-tree (flightRouterPath)
-  // and a root layout isn't already present views/layout.js or
-  // views/(new-root)/layout.js
-  let hasRootLayout = componentPaths.some(
-    (path) => path === '/' || path.match(/\/\(.*?\)$/)
-  )
   const isSubtreeRender = components.length < componentPaths.length
 
-  if (!hasRootLayout && !isSubtreeRender) {
-    components.unshift({ Component: DefaultRootLayout })
-    hasRootLayout = true
-  }
-
   // Reads of this are cached on the `req` object, so this should resolve
   // instantly. There's no need to pass this data down from a previous
   // invoke, where we'd have to consider server & serverless.
@@ -340,7 +327,7 @@ export async function renderToHTML(
       }
 
       // if this is the root layout pass children as bodyChildren prop
-      if (hasRootLayout && i === 0) {
+      if (!isSubtreeRender && i === 0) {
         return React.createElement(layout.Component, {
           ...props,
           headChildren: props.headChildren,
@@ -369,7 +356,7 @@ export async function renderToHTML(
     // }
   }
 
-  const headChildren = hasRootLayout
+  const headChildren = !isSubtreeRender
     ? buildManifest.rootMainFiles.map((src) => (
         <script src={'/_next/' + src} async key={src} />
       ))

diff --git a/test/e2e/views-dir/app/views/(newroot)/dashboard/another/page.server.js b/test/e2e/views-dir/app/views/(newroot)/dashboard/another/page.server.js
@@ -0,0 +1,7 @@
+export default function AnotherPage(props) {
+  return (
+    <>
+      <p>hello from newroot/dashboard/another</p>
+    </>
+  )
+}
diff --git a/test/e2e/views-dir/app/views/(newroot)/layout.server.js b/test/e2e/views-dir/app/views/(newroot)/layout.server.js
@@ -0,0 +1,19 @@
+export async function getServerSideProps() {
+  return {
+    props: {
+      world: 'world',
+    },
+  }
+}
+
+export default function Root({ headChildren, bodyChildren, world }) {
+  return (
+    <html className="this-is-another-document-html">
+      <head>
+        {headChildren}
+        <title>{`hello ${world}`}</title>
+      </head>
+      <body className="this-is-another-document-body">{bodyChildren}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/views-dir/app/views/dashboard/(custom)/deployments/breakdown/page.server.js b/test/e2e/views-dir/app/views/dashboard/(custom)/deployments/breakdown/page.server.js
@@ -0,0 +1,7 @@
+export default function DeploymentsBreakdownPage(props) {
+  return (
+    <>
+      <p>hello from root/dashboard/(custom)/deployments/breakdown</p>
+    </>
+  )
+}
diff --git a/test/e2e/views-dir/app/views/dashboard/(custom)/layout.server.js b/test/e2e/views-dir/app/views/dashboard/(custom)/layout.server.js
@@ -0,0 +1,8 @@
+export default function CustomDashboardRootLayout({ children }) {
+  return (
+    <>
+      <h2>Custom dashboard</h2>
+      {children}
+    </>
+  )
+}
diff --git a/test/e2e/views-dir/index.test.ts b/test/e2e/views-dir/index.test.ts
@@ -61,12 +61,12 @@ describe('views dir', () => {
     expect($('p').text()).toBe('hello from root/dashboard/integrations')
   })
 
-  // TODO: why is this routable but /should-not-serve-server.server.js
   it('should not include parent when not in parent directory with route in directory', async () => {
     const html = await renderViaHTTP(next.url, '/dashboard/hello')
     const $ = cheerio.load(html)
 
-    // new root has to provide it's own custom root layout
+    // new root has to provide it's own custom root layout or the default
+    // is used instead
     expect(html).toContain('<html>')
     expect(html).toContain('<body>')
     expect($('html').hasClass('this-is-the-document-html')).toBeFalsy()
@@ -79,6 +79,44 @@ describe('views dir', () => {
     expect($('p').text()).toBe('hello from root/dashboard/rootonly/hello')
   })
 
+  it('should use new root layout when provided', async () => {
+    const html = await renderViaHTTP(next.url, '/dashboard/another')
+    const $ = cheerio.load(html)
+
+    // new root has to provide it's own custom root layout or the default
+    // is used instead
+    expect($('html').hasClass('this-is-another-document-html')).toBeTruthy()
+    expect($('body').hasClass('this-is-another-document-body')).toBeTruthy()
+
+    // Should not be nested in dashboard
+    expect($('h1').text()).toBeFalsy()
+
+    // Should render the page text
+    expect($('p').text()).toBe('hello from newroot/dashboard/another')
+  })
+
+  it('should not create new root layout when nested (optional)', async () => {
+    const html = await renderViaHTTP(
+      next.url,
+      '/dashboard/deployments/breakdown'
+    )
+    const $ = cheerio.load(html)
+
+    // new root has to provide it's own custom root layout or the default
+    // is used instead
+    expect($('html').hasClass('this-is-the-document-html')).toBeTruthy()
+    expect($('body').hasClass('this-is-the-document-body')).toBeTruthy()
+
+    // Should be nested in dashboard
+    expect($('h1').text()).toBe('Dashboard')
+    expect($('h2').text()).toBe('Custom dashboard')
+
+    // Should render the page text
+    expect($('p').text()).toBe(
+      'hello from root/dashboard/(custom)/deployments/breakdown'
+    )
+  })
+
   it('should include parent document when no direct parent layout', async () => {
     const html = await renderViaHTTP(next.url, '/dashboard/integrations')
     const $ = cheerio.load(html)