Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parallel routes: catch all nested under dynamic segment matches to the root #58272

Closed
1 task done
wlechowicz opened this issue Nov 9, 2023 · 10 comments · Fixed by #59791
Closed
1 task done

parallel routes: catch all nested under dynamic segment matches to the root #58272

wlechowicz opened this issue Nov 9, 2023 · 10 comments · Fixed by #59791
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@wlechowicz
Copy link

wlechowicz commented Nov 9, 2023

Link to the code that reproduces this issue

https://github.com/wlechowicz/next14-catchall-repro

To Reproduce

Caused by #58215, see #58215 (comment)

  1. Start the application with npm run dev
  2. Observe the error:
 ○ Compiling / ...
 ⨯ app\page.tsx
You cannot have two parallel pages that resolve to the same path. Please check /page and /[lang]/@modal/[...catchAll]/page. Refer to the route group docs for more information: https://nextjs.org/docs/app/building-your-application/routing/route-groups
  1. If the @modal, foo and bar are moved up to the app root (outside of [lang]), no such error appears and the catchAll closes the "modal" as expected when any /bar/* non-intercepted Link is clicked.

Current vs. Expected behavior

Previous behavior (prior to #58215) was that that [...catchAll] was not matching non-intercepted routes at all. See #50284
Current behavior with this change is that error quoted above is thrown at compile time.
Expected behavior is that catch-all matches to the correct parallel route, not root.

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:        
  Platform: win32        
  Arch: x64
  Version: Windows 10 Pro
Binaries:
  Node: 20.9.0
  npm: N/A
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.0.2-canary.27
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

App Router, Routing (next/router, next/navigation, next/link)

Additional context

No response

NEXT-1808

@wlechowicz wlechowicz added the bug Issue was opened via the bug report template. label Nov 9, 2023
@github-actions github-actions bot added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Nov 9, 2023
@mrjasonroy
Copy link

Can confirm, 14.0.2 broke my parallel modals:

src/app/product/[id]/page.tsx
You cannot have two parallel pages that resolve to the same path. Please check /product/[id]/page and /(home)/@modal/[...catchAll]/page. Refer to the route group docs for more information: https://nextjs.org/docs/app/building-your-application/routing/route-groups

Downgrading to 14.0.1 fixed the issue for me

@tangye1234
Copy link
Contributor

tangye1234 commented Nov 10, 2023

I also get this error: You cannot have two parallel pages that resolve to the same path. Please check /(article)/contact/page and /(main)/@sider/[...catchAll]/page

I think there is no need to check two parallel pages with different layout. because, the parallel routes can only be existed when it's layout dose not change to others.

The issue happened since 14.0.2

BTW, I found a parallel routes with only one default page, is not allowed since 14.0.2 which is a new bug.
The workaround is provide a not-found page with the same static page (which is not suitable for dynamic page).

@mrjasonroy
Copy link

Does anyone know if there is a workaround for 14.0.2 that can achieve the example modal? Or are we stuck on 14.0.1. The fix here would fix another issue I am have with default.tsx not being called in the base route of a parallel route in 14.0.1

@wlechowicz
Copy link
Author

@mrjasonroy pre- #58215 there were other issues, where the catchAll on a parallel route was not matching at all (at least combined with an intercepting route). For me it's the same as you, there isn't a version where the example modal works, because before it doesn't dismiss with a catchAll, after that PR it throws those errors.

@tangye1234
Copy link
Contributor

Does anyone know if there is a workaround for 14.0.2 that can achieve the example modal? Or are we stuck on 14.0.1. The fix here would fix another issue I am have with default.tsx not being called in the base route of a parallel route in 14.0.1

workaround: using not-found instead of default. The only issue is not-found has no dynamic params.

@wlechowicz
Copy link
Author

wlechowicz commented Nov 13, 2023

@tangye1234 @mrjasonroy It looks like this is fixed in 14.0.3-canary.6 by #58368 I assume. Until you toggle PPR on, then all hell breaks loose wrt parallel and intercepting routes.

cc @feedthejim

@mrjasonroy
Copy link

Still an error with me in 14.0.3. Maybe it has to do with how I am using parallel routes for slots and for things like modals, along with route groups for layouts.

I noticed that nested route groups don't work properly when using router.push() as well - I will investigate more and open an issue if I can figure out the core issue.

src/app/@header/chat/[id]/page.tsx
You cannot have two parallel pages that resolve to the same path. Please check /chat/[id]/page and //(home)/@modal/[...catchAll]/page

@feedthejim
Copy link
Contributor

I don't see an error on the latest canary @wlechowicz can you confirm? I think there's another bug though.

@enchorb
Copy link

enchorb commented Nov 18, 2023

+1, Getting this on 14.0.2+, fine on 14.0.1

@feedthejim feedthejim added the linear: next Confirmed issue that is tracked by the Next.js team. label Dec 6, 2023
ztanner added a commit that referenced this issue Dec 22, 2023
### What?
Catch-all routes are being matched to parallel routes which causes
issues like an interception route being handled by the wrong page
component, or a page component being associated with multiple pages
resulting in a "You cannot have two parallel pages that resolve to the
same path" build error.

### Why?
#58215 fixed a bug that caused catchall paths to not properly match when
used with parallel routes. In other words, a catchall slot wouldn't
render on a page that could match that catch all. Or a catchall page
wouldn't match a slot. At build time, a normalization step was
introduced to take application paths and attempt to perform this
matching behavior.

However in it's current form, this causes the errors mentioned above to
manifest. To better illustrate the problem, here are a few examples:

Given:
```
{
  '/': [ '/page' ],
  '/[...slug]': [ '/[...slug]/page' ],
  '/items/[...ids]': [ '/items/[...ids]/page' ],
  '/(.)items/[...ids]': [ '/@modal/(.)items/[...ids]/page' ]
}
```

The normalization logic would produce:
```
{
  '/': [ '/page' ],
  '/[...slug]': [ '/[...slug]/page' ],
  '/items/[...ids]': [ '/items/[...ids]/page' ],
  '/(.)items/[...ids]': [ '/@modal/(.)items/[...ids]/page', '/[...slug]/page' ]
}
```
The interception route will now be improperly handled by
`[...slug]/page` rather than the interception handler.

Another example, which rather than incorrectly handling a match, will
produce a build error:

Given:
```
{
  '/': [ '/(group-b)/page' ],
  '/[...catcher]': [ '/(group-a)/@parallel/[...catcher]/page' ]
}
```

The normalization logic would produce:

```
{
  '/': [ '/(group-b)/page', '/(group-a)/@parallel/[...catcher]/page' ],
  '/[...catcher]': [ '/(group-a)/@parallel/[...catcher]/page' ]
}
```
The parallel catch-all slot is now part of `/`. This means when building
the loader tree, two `children` parallel segments (aka page components)
will be found when hitting `/`, which is an error.

The error that was added here was originally intended to help catch
scenarios like:
`/app/(group-a)/page` and `/app/(group-b)/page`. However it also throws
for parallel slots, which isn't necessarily an error (especially since
the normalization logic will push potential matches).

### How?
There are two small fixes in this PR, the rest are an abundance of e2e
tests to help prevent regressions.

- When normalizing catch-all routes, we will not attempt to push any
page entrypoints for interception routes. These should already have all
the information they need in `appPaths`.
- Before throwing the error about duplicate page segments in
`next-app-loader`, we check to see if it's because we already matched a
page component but we also detected a parallel slot that would have
matched the page slot. In this case, we don't error, since the app can
recover from this.
- Loading a client reference manifest shouldn't throw a cryptic require
error. `loadClientReferenceManifest` is already potentially returning
undefined, so this case should already be handled gracefully

Separately, we'll need to follow-up on the Turbopack side to:
- Make sure the duplicate matching matches the Webpack implementation (I
believe Webpack is sorting, but Turbopack isn't)
- Implement #58215 in Turbopack. Once this is done, we should expect the
tests added in this PR to start failing.

Fixes #58272
Fixes #58660
Fixes #58312
Fixes #59782
Fixes #59784

Closes NEXT-1809
Copy link
Contributor

github-actions bot commented Jan 6, 2024

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Jan 6, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 6, 2024
agustints pushed a commit to agustints/next.js that referenced this issue Jan 6, 2024
### What?
Catch-all routes are being matched to parallel routes which causes
issues like an interception route being handled by the wrong page
component, or a page component being associated with multiple pages
resulting in a "You cannot have two parallel pages that resolve to the
same path" build error.

### Why?
vercel#58215 fixed a bug that caused catchall paths to not properly match when
used with parallel routes. In other words, a catchall slot wouldn't
render on a page that could match that catch all. Or a catchall page
wouldn't match a slot. At build time, a normalization step was
introduced to take application paths and attempt to perform this
matching behavior.

However in it's current form, this causes the errors mentioned above to
manifest. To better illustrate the problem, here are a few examples:

Given:
```
{
  '/': [ '/page' ],
  '/[...slug]': [ '/[...slug]/page' ],
  '/items/[...ids]': [ '/items/[...ids]/page' ],
  '/(.)items/[...ids]': [ '/@modal/(.)items/[...ids]/page' ]
}
```

The normalization logic would produce:
```
{
  '/': [ '/page' ],
  '/[...slug]': [ '/[...slug]/page' ],
  '/items/[...ids]': [ '/items/[...ids]/page' ],
  '/(.)items/[...ids]': [ '/@modal/(.)items/[...ids]/page', '/[...slug]/page' ]
}
```
The interception route will now be improperly handled by
`[...slug]/page` rather than the interception handler.

Another example, which rather than incorrectly handling a match, will
produce a build error:

Given:
```
{
  '/': [ '/(group-b)/page' ],
  '/[...catcher]': [ '/(group-a)/@parallel/[...catcher]/page' ]
}
```

The normalization logic would produce:

```
{
  '/': [ '/(group-b)/page', '/(group-a)/@parallel/[...catcher]/page' ],
  '/[...catcher]': [ '/(group-a)/@parallel/[...catcher]/page' ]
}
```
The parallel catch-all slot is now part of `/`. This means when building
the loader tree, two `children` parallel segments (aka page components)
will be found when hitting `/`, which is an error.

The error that was added here was originally intended to help catch
scenarios like:
`/app/(group-a)/page` and `/app/(group-b)/page`. However it also throws
for parallel slots, which isn't necessarily an error (especially since
the normalization logic will push potential matches).

### How?
There are two small fixes in this PR, the rest are an abundance of e2e
tests to help prevent regressions.

- When normalizing catch-all routes, we will not attempt to push any
page entrypoints for interception routes. These should already have all
the information they need in `appPaths`.
- Before throwing the error about duplicate page segments in
`next-app-loader`, we check to see if it's because we already matched a
page component but we also detected a parallel slot that would have
matched the page slot. In this case, we don't error, since the app can
recover from this.
- Loading a client reference manifest shouldn't throw a cryptic require
error. `loadClientReferenceManifest` is already potentially returning
undefined, so this case should already be handled gracefully

Separately, we'll need to follow-up on the Turbopack side to:
- Make sure the duplicate matching matches the Webpack implementation (I
believe Webpack is sorting, but Turbopack isn't)
- Implement vercel#58215 in Turbopack. Once this is done, we should expect the
tests added in this PR to start failing.

Fixes vercel#58272
Fixes vercel#58660
Fixes vercel#58312
Fixes vercel#59782
Fixes vercel#59784

Closes NEXT-1809
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants