ensure @children slots work with catch-all routes

vercel · Jan 5, 2024 · 010defb · 010defb
1 parent d946c66
commit 010defb
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 3 deletions.
diff --git a/packages/next/src/build/normalize-catchall-routes.test.ts b/packages/next/src/build/normalize-catchall-routes.test.ts
@@ -96,4 +96,17 @@ describe('normalizeCatchallRoutes', () => {
       ],
     })
   })
+
+  it('should not add the catch-all route to a path that has a @children slot', async () => {
+    const appPaths = {
+      '/': ['/@children/page', '/@slot/page'],
+      '/[...slug]': ['/[...slug]/page'],
+      '/nested': ['/nested/@children/page'],
+    }
+
+    const initialAppPaths = JSON.parse(JSON.stringify(appPaths))
+    normalizeCatchAllRoutes(appPaths)
+
+    expect(appPaths).toMatchObject(initialAppPaths)
+  })
 })
diff --git a/packages/next/src/build/normalize-catchall-routes.ts b/packages/next/src/build/normalize-catchall-routes.ts
@@ -50,18 +50,29 @@ export function normalizeCatchAllRoutes(
 }
 
 function hasMatchedSlots(path1: string, path2: string): boolean {
-  const slots1 = path1.split('/').filter((segment) => segment.startsWith('@'))
-  const slots2 = path2.split('/').filter((segment) => segment.startsWith('@'))
+  const slots1 = path1.split('/').filter(isMatchableSlot)
+  const slots2 = path2.split('/').filter(isMatchableSlot)
 
+  // if the catch-all route does not have the same number of slots as the app path, it can't match
   if (slots1.length !== slots2.length) return false
 
+  // compare the slots in both paths. For there to be a match, each slot must be the same
   for (let i = 0; i < slots1.length; i++) {
     if (slots1[i] !== slots2[i]) return false
   }
 
   return true
 }
 
+/**
+ * Returns true for slots that should be considered when checking for match compatability.
+ * Excludes children slots because these are similar to having a segment-level `page`
+ * which would cause a slot length mismatch when comparing it to a catch-all route.
+ */
+function isMatchableSlot(segment: string): boolean {
+  return segment.startsWith('@') && segment !== '@children'
+}
+
 const catchAllRouteRegex = /\[?\[\.\.\./
 
 function isCatchAllRoute(pathname: string): boolean {

diff --git a/packages/next/src/build/webpack/plugins/next-types-plugin/index.ts b/packages/next/src/build/webpack/plugins/next-types-plugin/index.ts
@@ -170,7 +170,11 @@ export interface LayoutProps {
   children?: React.ReactNode
 ${
   options.slots
-    ? options.slots.map((slot) => `  ${slot}: React.ReactNode`).join('\n')
+    ? options.slots
+        // `@children` slots are matched to the children prop. If we re-add it here, it will cause a type error.
+        .filter((slot) => slot !== 'children')
+        .map((slot) => `  ${slot}: React.ReactNode`)
+        .join('\n')
     : ''
 }
   params?: any

diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@children/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@children/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Hello from @children/page</div>
+}
diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/default.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/default.tsx
@@ -0,0 +1,3 @@
+export default function Default() {
+  return <div>Default @slot</div>
+}
diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>@slot content</div>
+}
diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/[...catchAll]/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/[...catchAll]/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return 'root catchall'
+}
diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/layout.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/layout.tsx
@@ -0,0 +1,18 @@
+import React from 'react'
+
+export default function Root({
+  children,
+  slot,
+}: {
+  children: React.ReactNode
+  slot: React.ReactNode
+}) {
+  return (
+    <html>
+      <body>
+        <div id="children">{children}</div>
+        <div id="slot">{slot}</div>
+      </body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/@children/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/@children/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Hello from nested @children page</div>
+}
diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/layout.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/layout.tsx
@@ -0,0 +1,3 @@
+export default function Page({ children }) {
+  return <div>{children}</div>
+}
diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/next.config.js b/test/e2e/app-dir/parallel-routes-catchall-children-slot/next.config.js
@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig
diff --git a/...dir/parallel-routes-catchall-children-slot/parallel-routes-catchall-children-slot.test.ts b/...dir/parallel-routes-catchall-children-slot/parallel-routes-catchall-children-slot.test.ts
@@ -0,0 +1,28 @@
+import { createNextDescribe } from 'e2e-utils'
+
+createNextDescribe(
+  'parallel-routes-catchall-children-slot',
+  {
+    files: __dirname,
+  },
+  ({ next }) => {
+    it('should match the @children slot for a page before attempting to match the catchall', async () => {
+      let browser = await next.browser('/')
+      await expect(browser.elementById('children').text()).resolves.toBe(
+        'Hello from @children/page'
+      )
+      await expect(browser.elementById('slot').text()).resolves.toBe(
+        '@slot content'
+      )
+
+      browser = await next.browser('/nested')
+
+      await expect(browser.elementById('children').text()).resolves.toBe(
+        'Hello from nested @children page'
+      )
+      await expect(browser.elementById('slot').text()).resolves.toBe(
+        'Default @slot'
+      )
+    })
+  }
+)