-
Notifications
You must be signed in to change notification settings - Fork 26.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix parallel catch-all route normalization (#59791)
### 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
- Loading branch information
Showing
39 changed files
with
510 additions
and
29 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
import { normalizeCatchAllRoutes } from './normalize-catchall-routes' | ||
|
||
describe('normalizeCatchallRoutes', () => { | ||
it('should not add the catch-all to the interception route', () => { | ||
const appPaths = { | ||
'/': ['/page'], | ||
'/[...slug]': ['/[...slug]/page'], | ||
'/things/[...ids]': ['/things/[...ids]/page'], | ||
'/(.)things/[...ids]': ['/@modal/(.)things/[...ids]/page'], | ||
} | ||
|
||
const initialAppPaths = JSON.parse(JSON.stringify(appPaths)) | ||
|
||
normalizeCatchAllRoutes(appPaths) | ||
|
||
expect(appPaths).toMatchObject(initialAppPaths) | ||
}) | ||
|
||
it('should add the catch-all route to all matched paths when nested', () => { | ||
const appPaths = { | ||
'/parallel-nested-catchall': ['/parallel-nested-catchall/page'], | ||
'/parallel-nested-catchall/[...catchAll]': [ | ||
'/parallel-nested-catchall/[...catchAll]/page', | ||
'/parallel-nested-catchall/@slot/[...catchAll]/page', | ||
], | ||
'/parallel-nested-catchall/bar': ['/parallel-nested-catchall/bar/page'], | ||
'/parallel-nested-catchall/foo': [ | ||
'/parallel-nested-catchall/foo/page', | ||
'/parallel-nested-catchall/@slot/foo/page', | ||
], | ||
'/parallel-nested-catchall/foo/[id]': [ | ||
'/parallel-nested-catchall/foo/[id]/page', | ||
], | ||
'/parallel-nested-catchall/foo/[...catchAll]': [ | ||
'/parallel-nested-catchall/@slot/foo/[...catchAll]/page', | ||
], | ||
} | ||
|
||
normalizeCatchAllRoutes(appPaths) | ||
|
||
expect(appPaths).toMatchObject({ | ||
'/parallel-nested-catchall': ['/parallel-nested-catchall/page'], | ||
'/parallel-nested-catchall/[...catchAll]': [ | ||
'/parallel-nested-catchall/[...catchAll]/page', | ||
'/parallel-nested-catchall/@slot/[...catchAll]/page', | ||
], | ||
'/parallel-nested-catchall/bar': [ | ||
'/parallel-nested-catchall/bar/page', | ||
'/parallel-nested-catchall/@slot/[...catchAll]/page', // inserted | ||
], | ||
'/parallel-nested-catchall/foo': [ | ||
'/parallel-nested-catchall/foo/page', | ||
'/parallel-nested-catchall/@slot/foo/page', | ||
], | ||
'/parallel-nested-catchall/foo/[id]': [ | ||
'/parallel-nested-catchall/foo/[id]/page', | ||
'/parallel-nested-catchall/@slot/foo/[...catchAll]/page', // inserted | ||
], | ||
'/parallel-nested-catchall/foo/[...catchAll]': [ | ||
'/parallel-nested-catchall/@slot/foo/[...catchAll]/page', | ||
'/parallel-nested-catchall/[...catchAll]/page', // inserted | ||
], | ||
}) | ||
}) | ||
|
||
it('should add the catch-all route to all matched paths at the root', () => { | ||
const appPaths = { | ||
'/': ['/page'], | ||
'/[...catchAll]': ['/[...catchAll]/page', '/@slot/[...catchAll]/page'], | ||
'/bar': ['/bar/page'], | ||
'/foo': ['/foo/page', '/@slot/foo/page'], | ||
'/foo/[id]': ['/foo/[id]/page'], | ||
'/foo/[...catchAll]': ['/@slot/foo/[...catchAll]/page'], | ||
} | ||
|
||
normalizeCatchAllRoutes(appPaths) | ||
|
||
expect(appPaths).toMatchObject({ | ||
'/': [ | ||
'/page', | ||
'/@slot/[...catchAll]/page', // inserted | ||
], | ||
'/[...catchAll]': ['/[...catchAll]/page', '/@slot/[...catchAll]/page'], | ||
'/bar': [ | ||
'/bar/page', | ||
'/@slot/[...catchAll]/page', // inserted | ||
], | ||
'/foo': ['/foo/page', '/@slot/foo/page'], | ||
'/foo/[id]': [ | ||
'/foo/[id]/page', | ||
'/@slot/foo/[...catchAll]/page', // inserted | ||
], | ||
'/foo/[...catchAll]': [ | ||
'/@slot/foo/[...catchAll]/page', | ||
'/[...catchAll]/page', //inserted | ||
], | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
test/e2e/app-dir/conflicting-page-segments/app/(group-a)/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import Link from 'next/link' | ||
|
||
export default function Home() { | ||
return ( | ||
<div> | ||
Home <Link href="/foo">To /foo</Link> | ||
</div> | ||
) | ||
} |
9 changes: 9 additions & 0 deletions
9
test/e2e/app-dir/conflicting-page-segments/app/(group-b)/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import Link from 'next/link' | ||
|
||
export default function Home() { | ||
return ( | ||
<div> | ||
Home <Link href="/foo">To /foo</Link> | ||
</div> | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import React from 'react' | ||
|
||
export default function Root({ children }: { children: React.ReactNode }) { | ||
return ( | ||
<html> | ||
<body> | ||
<div id="children">{children}</div> | ||
</body> | ||
</html> | ||
) | ||
} |
31 changes: 31 additions & 0 deletions
31
test/e2e/app-dir/conflicting-page-segments/conflicting-page-segments.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import { createNextDescribe } from 'e2e-utils' | ||
import { check } from 'next-test-utils' | ||
|
||
createNextDescribe( | ||
'conflicting-page-segments', | ||
{ | ||
files: __dirname, | ||
// we skip start because the build will fail and we won't be able to catch it | ||
// start is re-triggered but caught in the assertions below | ||
skipStart: true, | ||
}, | ||
({ next, isNextDev }) => { | ||
it('should throw an error when a route groups causes a conflict with a parallel segment', async () => { | ||
if (isNextDev) { | ||
await next.start() | ||
const html = await next.render('/') | ||
|
||
expect(html).toContain( | ||
'You cannot have two parallel pages that resolve to the same path.' | ||
) | ||
} else { | ||
await expect(next.start()).rejects.toThrow('next build failed') | ||
|
||
await check( | ||
() => next.cliOutput, | ||
/You cannot have two parallel pages that resolve to the same path\. Please check \/\(group-a\)\/page and \/\(group-b\)\/page\./i | ||
) | ||
} | ||
}) | ||
} | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
/** | ||
* @type {import('next').NextConfig} | ||
*/ | ||
const nextConfig = {} | ||
|
||
module.exports = nextConfig |
3 changes: 3 additions & 0 deletions
3
test/e2e/app-dir/interception-routes-root-catchall/app/@modal/(.)items/[...ids]/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function Page({ params }: { params: { ids: string[] } }) { | ||
return <div>Intercepted Modal Page. Id: {params.ids}</div> | ||
} |
3 changes: 3 additions & 0 deletions
3
test/e2e/app-dir/interception-routes-root-catchall/app/@modal/default.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function Default() { | ||
return <div>default @modal</div> | ||
} |
3 changes: 3 additions & 0 deletions
3
test/e2e/app-dir/interception-routes-root-catchall/app/[...slug]/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function Page() { | ||
return <div>Root Catch-All Page</div> | ||
} |
3 changes: 3 additions & 0 deletions
3
test/e2e/app-dir/interception-routes-root-catchall/app/default.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function Default() { | ||
return <div>default root</div> | ||
} |
3 changes: 3 additions & 0 deletions
3
test/e2e/app-dir/interception-routes-root-catchall/app/items/[...ids]/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function Page({ params }: { params: { ids: string[] } }) { | ||
return <div>Regular Item Page. Id: {params.ids}</div> | ||
} |
18 changes: 18 additions & 0 deletions
18
test/e2e/app-dir/interception-routes-root-catchall/app/layout.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import React from 'react' | ||
|
||
export default function Root({ | ||
children, | ||
modal, | ||
}: { | ||
children: React.ReactNode | ||
modal: React.ReactNode | ||
}) { | ||
return ( | ||
<html> | ||
<body> | ||
<div id="children">{children}</div> | ||
<div id="slot">{modal}</div> | ||
</body> | ||
</html> | ||
) | ||
} |
10 changes: 10 additions & 0 deletions
10
test/e2e/app-dir/interception-routes-root-catchall/app/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import Link from 'next/link' | ||
|
||
export default async function Home() { | ||
return ( | ||
<div> | ||
<Link href="/items/1">Open Items #1 (Intercepted)</Link> | ||
<Link href="/foobar">Go to Catch-All Page</Link> | ||
</div> | ||
) | ||
} |
41 changes: 41 additions & 0 deletions
41
test/e2e/app-dir/interception-routes-root-catchall/interception-routes-root-catchall.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import { createNextDescribe } from 'e2e-utils' | ||
import { check } from 'next-test-utils' | ||
|
||
createNextDescribe( | ||
'interception-routes-root-catchall', | ||
{ | ||
files: __dirname, | ||
}, | ||
({ next }) => { | ||
it('should support having a root catch-all and a catch-all in a parallel route group', async () => { | ||
const browser = await next.browser('/') | ||
await browser.elementByCss('[href="/items/1"]').click() | ||
|
||
// this triggers the /items route interception handling | ||
await check( | ||
() => browser.elementById('slot').text(), | ||
/Intercepted Modal Page. Id: 1/ | ||
) | ||
await browser.refresh() | ||
|
||
// no longer intercepted, using the page | ||
await check(() => browser.elementById('slot').text(), /default @modal/) | ||
await check( | ||
() => browser.elementById('children').text(), | ||
/Regular Item Page. Id: 1/ | ||
) | ||
}) | ||
|
||
it('should handle non-intercepted catch-all pages', async () => { | ||
const browser = await next.browser('/') | ||
|
||
// there's no explicit page for /foobar. This will trigger the catchall [...slug] page | ||
await browser.elementByCss('[href="/foobar"]').click() | ||
await check(() => browser.elementById('slot').text(), /default @modal/) | ||
await check( | ||
() => browser.elementById('children').text(), | ||
/Root Catch-All Page/ | ||
) | ||
}) | ||
} | ||
) |
6 changes: 6 additions & 0 deletions
6
test/e2e/app-dir/interception-routes-root-catchall/next.config.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
/** | ||
* @type {import('next').NextConfig} | ||
*/ | ||
const nextConfig = {} | ||
|
||
module.exports = nextConfig |
Oops, something went wrong.