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: fix @children slots #60288

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/next/src/build/normalize-catchall-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 13 additions & 2 deletions packages/next/src/build/normalize-catchall-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,29 @@ 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
}

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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>Hello from @children/page</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return <div>Default @slot</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>@slot content</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return 'root catchall'
}
Original file line number Diff line number Diff line change
@@ -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>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>Hello from nested @children page</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page({ children }) {
return <div>{children}</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -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'
)
})
}
)
Loading