Multiple levels of catch-all routes with intercepted + parallel routes loads the wrong route on intercept, but the correct route on initial page load #58660
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
github-actions
bot
added
the
Navigation
|
I can confirm that the problem is the catch-all route. But in my case I don't have multiple levels of catch-all Removing the top catch-all route resolved the "not found" issue. I believe this has been broken in 14.0.3 with this PR #58368
|
feedthejim
added
the
linear: next
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
|
Fix works great (14.0.5-canary.24), thanks 🥳 |
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
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Link to the code that reproduces this issue
https://github.com/augustl/nextjs-intercepting-modal-bug
To Reproduce
Current vs. Expected behavior
I expected the click of "Show thing 1" or "Show thing 2" to load the intercepted route,
src/app/@modal/(.)things/[...ids]/page.tsx. Instead, it attempts to load the top-level catch all routesrc/app/[...slug]/page.tsx, as evident by the error message that refers to a line in that file caused by parameters being null due to the route loading mismatch.Also observe that when refreshing the page to go directly to localhost:3000/things/1, the page loads correctly and without error.
Verify canary release
Provide environment information
Operating System: Platform: darwin Arch: arm64 Version: Darwin Kernel Version 23.1.0: Mon Oct 9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 Binaries: Node: 18.18.2 npm: 9.8.1 Yarn: 1.22.21 pnpm: N/A Relevant Packages: next: 14.0.4-canary.4 eslint-config-next: 14.0.3 react: 18.2.0 react-dom: 18.2.0 typescript: 5.2.2 Next.js Config: output: N/AWhich area(s) are affected? (Select all that apply)
App Router, Routing (next/router, next/navigation, next/link)
Additional context
The combination of a top level catch-all route and a catch-all segment in the
/things/[...ids]route are both needed to trigger this issue. If you remove the top level catch-all[...slug], or change/things/[...ids]to something like/things/[id], it works fine.NEXT-1809
The text was updated successfully, but these errors were encountered: