Skip to content

Commit

Permalink
Fix per-entry client reference manifest for grouped and named segments (
Browse files Browse the repository at this point in the history
#52664)

References for Client Components need to be aggregated to the page entry level, and emitted as files in the correct directory for the SSR server to read from. For normal routes (e.g. `app/foo/bar/page`), we can go through and collect all entries (layout, loading, error, ...) from the current and parent segments, to aggregate all necessary client references.

However, for routes with special conventions like `app/(group)/@named/foo/bar/page`, it needs to be normalized (remove the named slot and group segments) so it can be grouped together with `app/(group)/@named2/foo/bar/loading`.
  • Loading branch information
shuding committed Jul 14, 2023
1 parent 79227ee commit b957f52
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 71 deletions.
123 changes: 52 additions & 71 deletions packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts
Expand Up @@ -155,6 +155,7 @@ export class ClientReferenceManifestPlugin {
context: string
) {
const manifestsPerGroup = new Map<string, ClientReferenceManifest[]>()
const manifestEntryFiles: string[] = []

compilation.chunkGroups.forEach((chunkGroup) => {
// By default it's the shared chunkGroup (main-app) for every page.
Expand Down Expand Up @@ -334,94 +335,74 @@ export class ClientReferenceManifestPlugin {
// A page's entry name can have extensions. For example, these are both valid:
// - app/foo/page
// - app/foo/page.page
// Let's normalize the entry name to remove the extra extension
const groupName = /\/page(\.[^/]+)?$/.test(entryName)
? entryName.replace(/\/page(\.[^/]+)?$/, '/page')
: entryName.slice(0, entryName.lastIndexOf('/'))
if (/\/page(\.[^/]+)?$/.test(entryName)) {
manifestEntryFiles.push(entryName.replace(/\/page(\.[^/]+)?$/, '/page'))
}

// Special case for the root not-found page.
if (/^app\/not-found(\.[^.]+)?$/.test(entryName)) {
manifestEntryFiles.push('app/not-found')
}

// Group the entry by their route path, so the page has all manifest items
// it needs:
// - app/foo/loading -> app/foo
// - app/foo/page -> app/foo
// - app/(group)/@named/foo/page -> app/foo
const groupName = entryName
.slice(0, entryName.lastIndexOf('/'))
.replace(/\/@[^/]+/g, '')
.replace(/\/\([^/]+\)/g, '')

if (!manifestsPerGroup.has(groupName)) {
manifestsPerGroup.set(groupName, [])
}
manifestsPerGroup.get(groupName)!.push(manifest)
})

if (entryName.includes('/@')) {
// Remove parallel route labels:
// - app/foo/@bar/page -> app/foo
// - app/foo/@bar/layout -> app/foo/layout -> app/foo
const entryNameWithoutNamedSegments = entryName.replace(/\/@[^/]+/g, '')
const groupNameWithoutNamedSegments =
entryNameWithoutNamedSegments.slice(
0,
entryNameWithoutNamedSegments.lastIndexOf('/')
)
if (!manifestsPerGroup.has(groupNameWithoutNamedSegments)) {
manifestsPerGroup.set(groupNameWithoutNamedSegments, [])
}
manifestsPerGroup.get(groupNameWithoutNamedSegments)!.push(manifest)
}
// console.log(manifestEntryFiles, manifestsPerGroup)

// Special case for the root not-found page.
if (/^app\/not-found(\.[^.]+)?$/.test(entryName)) {
if (!manifestsPerGroup.has('app/not-found')) {
manifestsPerGroup.set('app/not-found', [])
}
manifestsPerGroup.get('app/not-found')!.push(manifest)
// Generate per-page manifests.
for (const pageName of manifestEntryFiles) {
const mergedManifest: ClientReferenceManifest = {
ssrModuleMapping: {},
edgeSSRModuleMapping: {},
clientModules: {},
entryCSSFiles: {},
}
})

// Generate per-page manifests.
for (const [groupName] of manifestsPerGroup) {
if (groupName.endsWith('/page') || groupName === 'app/not-found') {
const mergedManifest: ClientReferenceManifest = {
ssrModuleMapping: {},
edgeSSRModuleMapping: {},
clientModules: {},
entryCSSFiles: {},
}
const segments = pageName.split('/')
let group = ''
for (const segment of segments) {
if (segment.startsWith('@')) continue
if (segment.startsWith('(') && segment.endsWith(')')) continue

const segments = groupName.split('/')
let group = ''
for (const segment of segments) {
if (segment.startsWith('@')) continue
for (const manifest of manifestsPerGroup.get(group) || []) {
mergeManifest(mergedManifest, manifest)
}
group += (group ? '/' : '') + segment
}
for (const manifest of manifestsPerGroup.get(groupName) || []) {
for (const manifest of manifestsPerGroup.get(group) || []) {
mergeManifest(mergedManifest, manifest)
}
group += (group ? '/' : '') + segment
}

const json = JSON.stringify(mergedManifest)

const pagePath = groupName.replace(/%5F/g, '_')
const pageBundlePath = normalizePagePath(pagePath.slice('app'.length))
assets[
'server/app' +
pageBundlePath +
'_' +
CLIENT_REFERENCE_MANIFEST +
'.js'
] = new sources.RawSource(
`globalThis.__RSC_MANIFEST=(globalThis.__RSC_MANIFEST||{});globalThis.__RSC_MANIFEST[${JSON.stringify(
pagePath.slice('app'.length)
)}]=${JSON.stringify(json)}`
) as unknown as webpack.sources.RawSource

if (pagePath === 'app/not-found') {
// Create a separate special manifest for the root not-found page.
assets[
'server/' +
'app/_not-found' +
'_' +
CLIENT_REFERENCE_MANIFEST +
'.js'
] = new sources.RawSource(
const json = JSON.stringify(mergedManifest)

const pagePath = pageName.replace(/%5F/g, '_')
const pageBundlePath = normalizePagePath(pagePath.slice('app'.length))
assets[
'server/app' + pageBundlePath + '_' + CLIENT_REFERENCE_MANIFEST + '.js'
] = new sources.RawSource(
`globalThis.__RSC_MANIFEST=(globalThis.__RSC_MANIFEST||{});globalThis.__RSC_MANIFEST[${JSON.stringify(
pagePath.slice('app'.length)
)}]=${JSON.stringify(json)}`
) as unknown as webpack.sources.RawSource

if (pagePath === 'app/not-found') {
// Create a separate special manifest for the root not-found page.
assets['server/app/_not-found_' + CLIENT_REFERENCE_MANIFEST + '.js'] =
new sources.RawSource(
`globalThis.__RSC_MANIFEST=(globalThis.__RSC_MANIFEST||{});globalThis.__RSC_MANIFEST[${JSON.stringify(
'/_not-found'
)}]=${JSON.stringify(json)}`
) as unknown as webpack.sources.RawSource
}
}
}

Expand Down
@@ -0,0 +1,5 @@
'use client'

export function Foo() {
return <h2>it works</h2>
}
@@ -0,0 +1,5 @@
import { Foo } from './client'

export default function Page() {
return <Foo />
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/rsc-basic/app/(group)/conventions/layout.js
@@ -0,0 +1,3 @@
export default function Layout({ named }) {
return <div>{named}</div>
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/rsc-basic/rsc-basic.test.ts
Expand Up @@ -158,6 +158,11 @@ createNextDescribe(
expect(html).toContain('foo.client')
})

it('should create client reference successfully for all file conventions', async () => {
const html = await next.render('/conventions')
expect(html).toContain('it works')
})

it('should be able to navigate between rsc routes', async () => {
const browser = await next.browser('/root')

Expand Down

0 comments on commit b957f52

Please sign in to comment.