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

Fix per-entry client reference manifest for grouped and named segments #52664

Merged
merged 4 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
120 changes: 51 additions & 69 deletions packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts
Original file line number Diff line number Diff line change
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,75 @@ 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
shuding marked this conversation as resolved.
Show resolved Hide resolved
.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 json = JSON.stringify(mergedManifest)

const pagePath = groupName.replace(/%5F/g, '_')
const pageBundlePath = normalizePagePath(pagePath.slice('app'.length))
const pagePath = pageName.replace(/%5F/g, '_')
const pageBundlePath = normalizePagePath(pagePath.slice('app'.length))
assets[
'server/app' + pageBundlePath + '_' + CLIENT_REFERENCE_MANIFEST + '.js'
huozhi marked this conversation as resolved.
Show resolved Hide resolved
] = 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' +
pageBundlePath +
'_' +
CLIENT_REFERENCE_MANIFEST +
'.js'
'server/' + 'app/_not-found' + '_' + CLIENT_REFERENCE_MANIFEST + '.js'
] = new sources.RawSource(
`globalThis.__RSC_MANIFEST=(globalThis.__RSC_MANIFEST||{});globalThis.__RSC_MANIFEST[${JSON.stringify(
pagePath.slice('app'.length)
'/_not-found'
)}]=${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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use client'

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

export default function Page() {
return <Foo />
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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