fix parallel route top-level catch-all normalization logic to support…

… nested explicit (non-catchall) slot routes (#60776)

Fix NEXT-2165

### What?

Addresses the limitation of #60240, where a dummy `default` file is
required in parallel route child slot to prevent errors in dev server
rendering (`TypeError: Cannot read properties of undefined (reading
'clientModules')`) as well as errors in build and deploy (`Error:
ENOENT: no such file or directory, lstat
‘/vercel/path0/.next/server/app/parallel-route/[section]/@part/[partSlug]/page_client-reference-manifest.js’`)

Without the `default.tsx`, builds and deployments will fail with:

<img width="956" alt="CleanShot 2024-01-18 at 02 12 36@2x"
src="https://github.com/vercel/next.js/assets/179761/80ba61bd-6ec0-4b16-a393-dc9375227e19">

local dev server will also crash with:

<img width="986" alt="CleanShot 2024-01-18 at 02 13 19@2x"
src="https://github.com/vercel/next.js/assets/179761/cc500a32-b2f8-47b4-999e-e57cf5141b2f">

> TypeError: Cannot read properties of undefined (reading
'clientModules')


### Why?

Since `default.tsx` is not a compulsory when you have slot that are
specific and ends with a dynamic route segment, this PR extends support
so that it is possible mixing catch-all routes with specific
non-catchall routes without requiring an additional `default.tsx` .

This PR will allow the following test cases to pass:

```
it('should not add the catch-all route to segments that have a more specific [dynamicRoute]', () => {
    const appPaths = {
      '/': ['/page'],
      '/[[...catchAll]]': ['/[[...catchAll]]/page'],
      '/nested/[foo]/[bar]/default': [
        '/nested/[foo]/[bar]/default',
        '/nested/[foo]/[bar]/@slot0/default',
        '/nested/[foo]/[bar]/@slot2/default',
      ],
      '/nested/[foo]/[bar]': [
        '/nested/[foo]/[bar]/@slot0/page',
        '/nested/[foo]/[bar]/@slot1/page',
      ],
      '/nested/[foo]/[bar]/[baz]': [
        '/nested/[foo]/[bar]/@slot0/[baz]/page',
        '/nested/[foo]/[bar]/@slot1/[baz]/page',
      ],
      '/[locale]/nested/[foo]/[bar]/[baz]/[qux]': [
        '/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page',
      ],
    }

    const initialAppPaths = JSON.parse(JSON.stringify(appPaths))
    normalizeCatchAllRoutes(appPaths)
    expect(appPaths).toMatchObject(initialAppPaths)
  })
...
```

```it('should not add the catch-all route to segments that have a more specific [dynamicRoute]', () => {
    const appPaths = {
      '/': ['/page'],
      '/[[...catchAll]]': ['/[[...catchAll]]/page'],
      '/nested/[foo]/[bar]/default': [
        '/nested/[foo]/[bar]/default',
        '/nested/[foo]/[bar]/@slot0/default',
        '/nested/[foo]/[bar]/@slot2/default',
      ],
      '/nested/[foo]/[bar]': [
        '/nested/[foo]/[bar]/@slot0/page',
        '/nested/[foo]/[bar]/@slot1/page',
      ],
      '/nested/[foo]/[bar]/[baz]': [
        '/nested/[foo]/[bar]/@slot0/[baz]/page',
        '/nested/[foo]/[bar]/@slot1/[baz]/page',
      ],
      '/[locale]/nested/[foo]/[bar]/[baz]/[qux]': [
        '/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page',
      ],
    }
...
```

and allow parallel routes defined in this [code
repro](https://github.com/williamli/nextjs-NEXT-2165) to build.


![image](https://github.com/vercel/next.js/assets/179761/030f4fe1-3a27-41e5-bbd9-bc511f95e5d7)


### How?

`packages/next/src/build/normalize-catchall-routes.ts` is extended to
check `appPath` to see if it is:
1. the route is not a catchall
2. `isMoreSpecific` than the closest `catchAllRoute`.


where `isMoreSpecific` is defined as:

```

function isMoreSpecific(pathname: string, catchAllRoute: string): boolean {
  const pathnameDepth = pathname.split('/').length
  const catchAllRouteDepth = catchAllRoute.split('/').length - 1
  return pathnameDepth > catchAllRouteDepth
}

```

---------

Co-authored-by: Zack Tanner <zacktanner@gmail.com>

packages/next/src/build/normalize-catchall-routes.test.ts

@@ -128,6 +128,61 @@ describe('normalizeCatchallRoutes', () => {
     expect(appPaths).toMatchObject(initialAppPaths)
   })
 
+  it('should not add the catch-all route to segments that have a more specific [dynamicRoute]', () => {
+    const appPaths = {
+      '/': ['/page'],
+      '/[[...catchAll]]': ['/[[...catchAll]]/page'],
+      '/nested/[foo]/[bar]/default': [
+        '/nested/[foo]/[bar]/default',
+        '/nested/[foo]/[bar]/@slot0/default',
+        '/nested/[foo]/[bar]/@slot2/default',
+      ],
+      '/nested/[foo]/[bar]': [
+        '/nested/[foo]/[bar]/@slot0/page',
+        '/nested/[foo]/[bar]/@slot1/page',
+      ],
+      '/nested/[foo]/[bar]/[baz]': [
+        '/nested/[foo]/[bar]/@slot0/[baz]/page',
+        '/nested/[foo]/[bar]/@slot1/[baz]/page',
+      ],
+      '/[locale]/nested/[foo]/[bar]/[baz]/[qux]': [
+        '/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page',
+      ],
+    }
+
+    const initialAppPaths = JSON.parse(JSON.stringify(appPaths))
+
+    // normalize appPaths against catchAlls
+    normalizeCatchAllRoutes(appPaths)
+
+    // ensure values are correct after normalizing
+    expect(appPaths).toMatchObject(initialAppPaths)
+  })
+
+  it('should not add the catch-all route to non-catchall segments that are more specific', () => {
+    const appPaths = {
+      '/': ['/page'],
+      '/[locale]/[[...catchAll]]': ['/[locale]/[[...catchAll]]/page'],
+      '/[locale]/nested/default': [
+        '/[locale]/nested/default',
+        '/[locale]/nested/@slot0/default',
+        '/[locale]/nested/@slot1/default',
+      ],
+      '/[locale]/nested': ['/[locale]/nested/page'],
+      '/[locale]/nested/bar': ['/[locale]/nested/@slot0/bar/page'],
+      '/[locale]/nested/foo': ['/[locale]/nested/@slot0/foo/page'],
+      '/[locale]/nested/baz': ['/[locale]/nested/@slot1/baz/page'],
+    }
+
+    const initialAppPaths = JSON.parse(JSON.stringify(appPaths))
+
+    // normalize appPaths against catchAlls
+    normalizeCatchAllRoutes(appPaths)
+
+    // ensure values are correct after normalizing
+    expect(appPaths).toMatchObject(initialAppPaths)
+  })
+
   it('should not add the catch-all route to a path that has a @children slot', async () => {
     const appPaths = {
       '/': ['/@children/page', '/@slot/page'],

packages/next/src/build/normalize-catchall-routes.ts

@@ -36,15 +36,18 @@ export function normalizeCatchAllRoutes(
         0,
         normalizedCatchAllRoute.search(catchAllRouteRegex)
       )
+
       if (
         // check if the appPath could match the catch-all
         appPath.startsWith(normalizedCatchAllRouteBasePath) &&
         // check if there's not already a slot value that could match the catch-all
         !appPaths[appPath].some((path) =>
           hasMatchedSlots(path, catchAllRoute)
         ) &&
-        // check if the catch-all is not already matched by a default route
-        !appPaths[`${appPath}/default`]
+        // check if the catch-all is not already matched by a default route or page route
+        !appPaths[`${appPath}/default`] &&
+        // check if appPath is a catch-all OR is not more specific than the catch-all
+        (isCatchAllRoute(appPath) || !isMoreSpecific(appPath, catchAllRoute))
       ) {
         appPaths[appPath].push(catchAllRoute)
       }
@@ -81,3 +84,10 @@ const catchAllRouteRegex = /\[?\[\.\.\./
 function isCatchAllRoute(pathname: string): boolean {
   return pathname.includes('[...') || pathname.includes('[[...')
 }
+
+// test to see if a path is more specific than a catch-all route
+function isMoreSpecific(pathname: string, catchAllRoute: string): boolean {
+  const pathnameDepth = pathname.split('/').length
+  const catchAllRouteDepth = catchAllRoute.split('/').length - 1
+  return pathnameDepth > catchAllRouteDepth
+}

test/e2e/app-dir/parallel-routes-catchall-default/parallel-routes-catchall-default.test.ts

@@ -35,7 +35,7 @@ createNextDescribe(
         '/[locale]/nested/[foo]/[bar]/default.tsx'
       )
 
-      // however we do have a slot for the `[baz]` segment and so we expect that to no match
+      // however we do have a slot for the `[baz]` segment and so we expect that to match
       expect(await browser.elementById('slot').text()).toBe(
         '/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx'
       )

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/[[...catchAll]]/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/[[...catchAll]]/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/@slot0/[baz]/page.tsx

@@ -0,0 +1,3 @@
+export default function Page({ params }) {
+  return <div>/[locale]/nested/[foo]/[bar]/@slot0/[baz]/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/@slot0/default.tsx

@@ -0,0 +1,3 @@
+export default function Page({ params }) {
+  return <div>/[locale]/nested/[foo]/[bar]/@slot0/default.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/@slot0/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/[foo]/[bar]/@slot0/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page.tsx

@@ -0,0 +1,3 @@
+export default function Page({ params }) {
+  return <div>/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/@slot1/[baz]/page.tsx

@@ -0,0 +1,3 @@
+export default function Page({ params }) {
+  return <div>/[locale]/nested/[foo]/[bar]/@slot1/[baz]/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/@slot1/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/[foo]/[bar]/@slot1/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/@slot2/default.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/[foo]/[bar]/@slot2/default.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/default.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/[foo]/[bar]/default.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/[locale]/nested/[foo]/[bar]/layout.tsx

@@ -0,0 +1,10 @@
+export default function Layout({ children, slot0, slot1, slot2 }) {
+  return (
+    <>
+      Children: <div id="nested-children">{children}</div>
+      Slot0: <div id="slot0">{slot0}</div>
+      Slot1: <div id="slot1">{slot1}</div>
+      Slot2: <div id="slot2">{slot2}</div>
+    </>
+  )
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/layout.tsx

@@ -0,0 +1,11 @@
+import React from 'react'
+
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <body>
+        Children: <div id="children">{children}</div>
+      </body>
+    </html>
+  )
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/app/page.tsx

@@ -0,0 +1,3 @@
+export default async function Home() {
+  return <div>Root Page</div>
+}

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/next.config.js

@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig

test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/parallel-routes-catchall-dynamic-segment.test.ts

@@ -0,0 +1,87 @@
+import { createNextDescribe } from 'e2e-utils'
+
+createNextDescribe(
+  'parallel-routes-catchall-dynamic-segment',
+  {
+    files: __dirname,
+  },
+  ({ next }) => {
+    it('should match default and dynamic segment paths before catch-all', async () => {
+      let browser = await next.browser('/en/nested')
+
+      // we have a top-level catch-all but the /nested dir doesn't have a default/page until the /[foo]/[bar] segment
+      // so we expect the top-level catch-all to render
+      expect(await browser.elementById('children').text()).toBe(
+        '/[locale]/[[...catchAll]]/page.tsx'
+      )
+
+      browser = await next.browser('/en/nested/foo/bar')
+
+      // we're now at the /[foo]/[bar] segment, so we expect the matched page to be the default (since there's no page defined)
+      expect(await browser.elementById('nested-children').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/default.tsx'
+      )
+
+      // we expect the slot0 to match since there's a page defined at this segment
+      expect(await browser.elementById('slot0').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot0/page.tsx'
+      )
+
+      // we expect the slot1 to match since there's a page defined at this segment
+      expect(await browser.elementById('slot1').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot1/page.tsx'
+      )
+
+      // we expect the slot2 to match since there's a default page defined at this segment
+      expect(await browser.elementById('slot2').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot2/default.tsx'
+      )
+
+      browser = await next.browser('/en/nested/foo/bar/baz')
+
+      // the page slot should still be the one matched at the /[foo]/[bar] segment because it's the default and we
+      // didn't define a page at the /[foo]/[bar]/[baz] segment
+      expect(await browser.elementById('nested-children').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/default.tsx'
+      )
+
+      // we do have a slot for the `[baz]` dynamic segment in slot0 and so we expect that to match
+      expect(await browser.elementById('slot0').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot0/[baz]/page.tsx'
+      )
+
+      // we do have a slot for the `[baz]` dynamic segment in slot1 and so we expect that to match
+      expect(await browser.elementById('slot1').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot1/[baz]/page.tsx'
+      )
+
+      // we do not have a slot for the `[baz]` dynamic segment in slot2 and so the default page is matched
+      expect(await browser.elementById('slot2').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot2/default.tsx'
+      )
+
+      browser = await next.browser('/en/nested/foo/bar/baz/qux')
+
+      // the page slot should still be the one matched at the /[foo]/[bar] segment because it's the default and we
+      // didn't define a page at the /[foo]/[bar]/[baz]/[qux] segment
+      expect(await browser.elementById('nested-children').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/default.tsx'
+      )
+
+      // we do not have a slot for the `[baz]/[qux]` dynamic segment in slot0 and so we expect the default page at `@slot0/` to be returned
+      expect(await browser.elementById('slot0').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot0/default.tsx'
+      )
+
+      // we do have a slot for the `[baz]/[qux]` dynamic segment in slot1 and so we expect that to no match
+      expect(await browser.elementById('slot1').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page.tsx'
+      )
+
+      // we do not have a slot for the `[baz]/[qux]` dynamic segment in slot2 and so we expect the default page at `@slot2/` to be returned
+      expect(await browser.elementById('slot2').text()).toBe(
+        '/[locale]/nested/[foo]/[bar]/@slot2/default.tsx'
+      )
+    })
+  }
+)

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/[[...catchAll]]/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/[[...catchAll]]/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/@slot0/bar/page.tsx

@@ -0,0 +1,3 @@
+export default function Page({ params }) {
+  return <div>/[locale]/nested/@slot0/bar/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/@slot0/default.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/@slot0/default.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/@slot0/foo/page.tsx

@@ -0,0 +1,3 @@
+export default function Page({ params }) {
+  return <div>/[locale]/nested/@slot0/foo/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/@slot1/baz/page.tsx

@@ -0,0 +1,3 @@
+export default function Page({ params }) {
+  return <div>/[locale]/nested/@slot1/baz/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/@slot1/default.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/@slot1/default.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/default.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/default.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/layout.tsx

@@ -0,0 +1,9 @@
+export default function Layout({ children, slot0, slot1 }) {
+  return (
+    <>
+      Children: <div id="nested-children">{children}</div>
+      Slot0: <div id="slot0">{slot0}</div>
+      Slot1: <div id="slot1">{slot1}</div>
+    </>
+  )
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/[locale]/nested/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>/[locale]/nested/page.tsx</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/layout.tsx

@@ -0,0 +1,11 @@
+import React from 'react'
+
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <body>
+        Children: <div id="children">{children}</div>
+      </body>
+    </html>
+  )
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/app/page.tsx

@@ -0,0 +1,3 @@
+export default async function Home() {
+  return <div>Root Page</div>
+}

test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchalls/next.config.js

@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig