Commit graph

2 commits

Author SHA1 Message Date
Willi#m ⬣
71335a9912
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>
2024-01-23 22:11:13 +00:00
Zack Tanner
90f95399dd
fix catch-all route normalization for default parallel routes (#60240)
### What?
When relying on a `default` route as a fallback, greedier catch-all
segments in the application hierarchy would take precedence, causing
unexpected errors/matching behavior.

### Why?
When performing parallel route catch-all normalization, we push
potential catch-all matches to paths without considering that a path
might instead be matched by a `default` page. Because of this, the
catch-all take precedence and the app will not try and load the default.

For example, given this structure:

```
{
  "/": ["/page"],
  "/[[...catchAll]]": ["/[[...catchAll]]/page"],
  "/nested/[foo]/[bar]": ["/nested/[foo]/[bar]/@slot/page"],
  "/nested/[foo]/[bar]/[baz]": ["/nested/[foo]/[bar]/@slot/[baz]/page"],
}
```

(Where there's a `/nested/[foo]/[bar]/default.tsx`)

The route normalization logic would produce:

```
{
  "/": ["/page"],
  "/[[...catchAll]]": ["/[[...catchAll]]/page"],
  "/nested/[foo]/[bar]": [
    "/nested/[foo]/[bar]/@slot/page",
    "/[[...catchAll]]/page",
  ],
  "/nested/[foo]/[bar]/[baz]": [
    "/nested/[foo]/[bar]/@slot/[baz]/page",
    "/[[...catchAll]]/page",
  ],
}
```
This means that when building the `LoaderTree`, it won't ever try to
find the default for that segment. **This solution operates under the
assumption that if you defined a `default` at a particular layout
segment, you intend for that to render in place of a greedier
catch-all.** (Let me know if this is an incorrect assumption)

### How?
We can't safely normalize catch-all parallel routes without having
context about where the `default` segments are, so this updates
`appPaths` to be inclusive of default segments and then filters them
when doing anything relating to build/export to maintain existing
behavior. We use this information to check if an existing default exists
at the same segment level that we'd push the catch-all to. If one
exists, we don't push the catch-all. Otherwise we proceed as normal.

Closes NEXT-1987
2024-01-05 14:20:45 -08:00