Skip to content

Commit

Permalink
parallel routes: fix client reference manifest grouping for catch-all…
Browse files Browse the repository at this point in the history
… segments (#60482)

### What?
When using catch-all routes in conjunction with parallel routes, and
when importing a client component (`"use client"`), the build would fail
with the following error:

> Could not find the module "PathToClientComponent" in the React Client
Manifest. This is probably a bug in the React Server Components bundler.

### Why?
`flight-manifest-plugin` generates manifests for each page entry. The
`clientModules` portion of this manifest is used by React to load the
appropriate client module. When React attempts to render a component
tree and detects a module that it cannot find, it will throw this error.
To illustrate why it isn't in the tree, consider the following example:

```
app
  page.tsx
  layout.tsx
  @slot
    [...catchAll]
      page.tsx
```

```tsx
// app/layout.tsx
export default function Layout({children, slot}) {
  return <>{children} {slot}</>
}
```
```tsx
// app/@slot/[...catchAll]/page.tsx
import Link from 'next/link'
export default function Page() {
  return <Link href="/">Test</Link>
}
```

When visiting `/`, we'd expect both the catch-all `@slot` and the root
page to render. At build time, we'll generate a client reference
manifest for `/` and `/[...catchAll]` since both are page components.
However, the `@slot` imports a client component. When we attempt to load
the client reference manifest for `/`, it will ignore the catch-all
slot's manifest, resulting in the error.

### How?
The `entryNameToGroupName` function seems to already exist to handle
this scenario for other cases. For example,
`app/(group)/@named/foo/page` needs to know about any manifests
associated with `app/foo`. This updates the code to apply similar
handling to catchAll segments. When applying this change to the example
mentioned earlier, it will properly merge the manifests for both
`app/@slot/[...catchAll]/page.tsx` and `app/page.tsx` because both will
be part of the `/` group.

Closes NEXT-1908
Fixes #59747
Fixes #59510
  • Loading branch information
ztanner authored Jan 10, 2024
1 parent b161872 commit d6c754f
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,19 @@ function getAppPathRequiredChunks(
// - app/foo/page -> app/foo
// - app/(group)/@named/foo/page -> app/foo
// - app/(.)foo/(..)bar/loading -> app/bar
// - app/[...catchAll]/page -> app
// - app/foo/@slot/[...catchAll]/page -> app/foo
function entryNameToGroupName(entryName: string) {
let groupName = entryName
.slice(0, entryName.lastIndexOf('/'))
// Remove slots
.replace(/\/@[^/]+/g, '')
// Remove the group with lookahead to make sure it's not interception route
.replace(/\/\([^/]+\)(?=(\/|$))/g, '')
// Remove catch-all routes since they should be part of the parent group that the catch-all would apply to.
// This is necessary to support parallel routes since multiple page components can be rendered on the same page.
// In order to do that, we need to ensure that the manifests are merged together by putting them in the same group.
.replace(/\/\[?\[\.\.\.[^\]]*\]\]?/g, '')

// Interception routes
groupName = groupName
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use client'

export function ClientComponent() {
return <div>catchall page client component</div>
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import { ClientComponent } from './client-component'

export default function Page() {
return 'main catchall'
return (
<div>
main catchall <ClientComponent />
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,11 @@ createNextDescribe(
await browser.elementByCss('[href="/parallel-catchall/baz"]').click()
await check(
() => browser.waitForElementByCss('#main').text(),
'main catchall'
/main catchall/
)
await check(
() => browser.waitForElementByCss('#main').text(),
/catchall page client component/
)
await check(
() => browser.waitForElementByCss('#slot-content').text(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use client'

export function ClientComponent() {
return <div>catchall slot client component</div>
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import { ClientComponent } from './client-component'

export default function Page() {
return 'slot catchall'
return (
<div>
slot catchall <ClientComponent />
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,19 @@ createNextDescribe(
// so we'd expect to see the catch-all slot & the page content
await check(() => browser.elementById('children').text(), /bar/)
await check(() => browser.elementById('slot').text(), /slot catchall/)
await check(
() => browser.elementById('slot').text(),
/catchall slot client component/
)
})

it('should match correctly when defining an explicit slot but no page', async () => {
const browser = await next.browser('/')
await check(() => browser.elementById('slot').text(), /slot catchall/)
await check(
() => browser.elementById('slot').text(),
/catchall slot client component/
)

await browser.elementByCss('[href="/baz"]').click()

Expand All @@ -46,12 +54,19 @@ createNextDescribe(
it('should match both the catch-all page & slot', async () => {
const browser = await next.browser('/')
await check(() => browser.elementById('slot').text(), /slot catchall/)

await check(
() => browser.elementById('slot').text(),
/catchall slot client component/
)
await browser.elementByCss('[href="/quux"]').click()

// quux doesn't have a page or slot defined. It should use the catch-all for both
await check(() => browser.elementById('children').text(), /main catchall/)
await check(() => browser.elementById('slot').text(), /slot catchall/)
await check(
() => browser.elementById('slot').text(),
/catchall slot client component/
)
})
}
)

0 comments on commit d6c754f

Please sign in to comment.