parallel routes: fix @children slots (#60288)

### What?
Our
[docs](https://nextjs.org/docs/app/building-your-application/routing/parallel-routes#convention)
point out that `app/page.js` is equivalent to `app/@children/page.js`,
however in practice this is not the case, and causes type errors when
using `@children` slots as well as incorrect behavior when matching
catch-all routes.

### Why?
- When typechecking, `@children` slots would be added to the typeguard
file for the associated layout, resulting in duplicate identifiers for
the `children` prop
- When determining where to insert catchall slots, the `hasMatchedSlots`
check wasn't considering that the `@children` slot corresponds with the
page component, so matching another page would clobber the previous one.

### How?
- Filters out the `@children` slot when collecting slots for
typechecking
- Filters out the `@children` slot when running the `hasMatchedSlots`
function in the catch-all normalizer

Closes NEXT-1984
This commit is contained in:
Zack Tanner 2024-01-06 07:24:44 -08:00 committed by GitHub
parent 43410c9926
commit efebba80a7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 100 additions and 3 deletions

View file

@ -115,6 +115,17 @@ describe('normalizeCatchallRoutes', () => {
const initialAppPaths = JSON.parse(JSON.stringify(appPaths))
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'],
'/[...slug]': ['/[...slug]/page'],
'/nested': ['/nested/@children/page'],
}
const initialAppPaths = JSON.parse(JSON.stringify(appPaths))
normalizeCatchAllRoutes(appPaths)
expect(appPaths).toMatchObject(initialAppPaths)

View file

@ -53,11 +53,13 @@ export function normalizeCatchAllRoutes(
}
function hasMatchedSlots(path1: string, path2: string): boolean {
const slots1 = path1.split('/').filter((segment) => segment.startsWith('@'))
const slots2 = path2.split('/').filter((segment) => segment.startsWith('@'))
const slots1 = path1.split('/').filter(isMatchableSlot)
const slots2 = path2.split('/').filter(isMatchableSlot)
// if the catch-all route does not have the same number of slots as the app path, it can't match
if (slots1.length !== slots2.length) return false
// compare the slots in both paths. For there to be a match, each slot must be the same
for (let i = 0; i < slots1.length; i++) {
if (slots1[i] !== slots2[i]) return false
}
@ -65,6 +67,15 @@ function hasMatchedSlots(path1: string, path2: string): boolean {
return true
}
/**
* Returns true for slots that should be considered when checking for match compatability.
* Excludes children slots because these are similar to having a segment-level `page`
* which would cause a slot length mismatch when comparing it to a catch-all route.
*/
function isMatchableSlot(segment: string): boolean {
return segment.startsWith('@') && segment !== '@children'
}
const catchAllRouteRegex = /\[?\[\.\.\./
function isCatchAllRoute(pathname: string): boolean {

View file

@ -213,7 +213,12 @@ async function collectNamedSlots(layoutPath: string) {
const items = await fs.readdir(layoutDir, { withFileTypes: true })
const slots = []
for (const item of items) {
if (item.isDirectory() && item.name.startsWith('@')) {
if (
item.isDirectory() &&
item.name.startsWith('@') &&
// `@children slots are matched to the children prop, and should not be handled separately for type-checking
item.name !== '@children'
) {
slots.push(item.name.slice(1))
}
}

View file

@ -0,0 +1,3 @@
export default function Page() {
return <div>Hello from @children/page</div>
}

View file

@ -0,0 +1,3 @@
export default function Default() {
return <div>Default @slot</div>
}

View file

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

View file

@ -0,0 +1,3 @@
export default function Page() {
return 'root catchall'
}

View file

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

View file

@ -0,0 +1,3 @@
export default function Page() {
return <div>Hello from nested @children page</div>
}

View file

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

View file

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

View file

@ -0,0 +1,28 @@
import { createNextDescribe } from 'e2e-utils'
createNextDescribe(
'parallel-routes-catchall-children-slot',
{
files: __dirname,
},
({ next }) => {
it('should match the @children slot for a page before attempting to match the catchall', async () => {
let browser = await next.browser('/')
await expect(browser.elementById('children').text()).resolves.toBe(
'Hello from @children/page'
)
await expect(browser.elementById('slot').text()).resolves.toBe(
'@slot content'
)
browser = await next.browser('/nested')
await expect(browser.elementById('children').text()).resolves.toBe(
'Hello from nested @children page'
)
await expect(browser.elementById('slot').text()).resolves.toBe(
'Default @slot'
)
})
}
)