From b957f52be3987abe958254bc5fa27bf04ffbeb6d Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Fri, 14 Jul 2023 03:18:37 +0200 Subject: [PATCH] Fix per-entry client reference manifest for grouped and named segments (#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`. --- .../webpack/plugins/flight-manifest-plugin.ts | 123 ++++++++---------- .../app/(group)/conventions/@named/client.js | 5 + .../app/(group)/conventions/@named/page.js | 5 + .../app/(group)/conventions/layout.js | 3 + test/e2e/app-dir/rsc-basic/rsc-basic.test.ts | 5 + 5 files changed, 70 insertions(+), 71 deletions(-) create mode 100644 test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/client.js create mode 100644 test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/page.js create mode 100644 test/e2e/app-dir/rsc-basic/app/(group)/conventions/layout.js diff --git a/packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts index 7b7d246a9a..9707072db9 100644 --- a/packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts +++ b/packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts @@ -155,6 +155,7 @@ export class ClientReferenceManifestPlugin { context: string ) { const manifestsPerGroup = new Map() + const manifestEntryFiles: string[] = [] compilation.chunkGroups.forEach((chunkGroup) => { // By default it's the shared chunkGroup (main-app) for every page. @@ -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) - } - - // 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 [groupName] of manifestsPerGroup) { - if (groupName.endsWith('/page') || groupName === 'app/not-found') { - const mergedManifest: ClientReferenceManifest = { - ssrModuleMapping: {}, - edgeSSRModuleMapping: {}, - clientModules: {}, - entryCSSFiles: {}, - } + // console.log(manifestEntryFiles, manifestsPerGroup) - 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) || []) { + // Generate per-page manifests. + for (const pageName of manifestEntryFiles) { + 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 + + 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)) - 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 + 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( + 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 - } } } diff --git a/test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/client.js b/test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/client.js new file mode 100644 index 0000000000..bc266d8894 --- /dev/null +++ b/test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/client.js @@ -0,0 +1,5 @@ +'use client' + +export function Foo() { + return

it works

+} diff --git a/test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/page.js b/test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/page.js new file mode 100644 index 0000000000..b4bddb9eab --- /dev/null +++ b/test/e2e/app-dir/rsc-basic/app/(group)/conventions/@named/page.js @@ -0,0 +1,5 @@ +import { Foo } from './client' + +export default function Page() { + return +} diff --git a/test/e2e/app-dir/rsc-basic/app/(group)/conventions/layout.js b/test/e2e/app-dir/rsc-basic/app/(group)/conventions/layout.js new file mode 100644 index 0000000000..cbaaeb4155 --- /dev/null +++ b/test/e2e/app-dir/rsc-basic/app/(group)/conventions/layout.js @@ -0,0 +1,3 @@ +export default function Layout({ named }) { + return
{named}
+} diff --git a/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts b/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts index 3e71e1d12d..2465e6d6a6 100644 --- a/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts +++ b/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts @@ -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')