From 80b0515cb4bea5a6922b9ad96f00499be910edf3 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 20 Apr 2021 20:18:21 +0200 Subject: [PATCH 1/4] improve react-loadable-plugin Co-authored-by: JJ Kasper --- .../babel/plugins/react-loadable-plugin.ts | 17 +- .../webpack/plugins/react-loadable-plugin.ts | 150 ++++++++++++------ packages/next/next-server/lib/utils.ts | 5 +- .../next-server/server/load-components.ts | 5 +- packages/next/next-server/server/render.tsx | 21 ++- packages/next/pages/_document.tsx | 37 ++--- .../next-dynamic/components/four.js | 3 + test/integration/next-dynamic/pages/index.js | 4 + .../next-dynamic/test/index.test.js | 2 +- 9 files changed, 152 insertions(+), 92 deletions(-) create mode 100644 test/integration/next-dynamic/components/four.js diff --git a/packages/next/build/babel/plugins/react-loadable-plugin.ts b/packages/next/build/babel/plugins/react-loadable-plugin.ts index 8ffb5aa3458e6..70d05569e0a22 100644 --- a/packages/next/build/babel/plugins/react-loadable-plugin.ts +++ b/packages/next/build/babel/plugins/react-loadable-plugin.ts @@ -36,7 +36,10 @@ export default function ({ }): PluginObj { return { visitor: { - ImportDeclaration(path: NodePath) { + ImportDeclaration( + path: NodePath, + state: any + ) { let source = path.node.source.value if (source !== 'next/dynamic') return @@ -133,7 +136,8 @@ export default function ({ if (!loader || Array.isArray(loader)) { return } - const dynamicImports: BabelTypes.StringLiteral[] = [] + const dynamicImports: BabelTypes.Expression[] = [] + const dynamicKeys: BabelTypes.Expression[] = [] loader.traverse({ Import(importPath) { @@ -141,6 +145,13 @@ export default function ({ if (!Array.isArray(importArguments)) return const node: any = importArguments[0].node dynamicImports.push(node) + dynamicKeys.push( + t.binaryExpression( + '+', + t.stringLiteral(state.file.opts.filename + ' -> '), + node + ) + ) }, }) @@ -169,7 +180,7 @@ export default function ({ ), t.objectProperty( t.identifier('modules'), - t.arrayExpression(dynamicImports) + t.arrayExpression(dynamicKeys) ), ]) ) diff --git a/packages/next/build/webpack/plugins/react-loadable-plugin.ts b/packages/next/build/webpack/plugins/react-loadable-plugin.ts index 52efe39040096..30fb3341357b6 100644 --- a/packages/next/build/webpack/plugins/react-loadable-plugin.ts +++ b/packages/next/build/webpack/plugins/react-loadable-plugin.ts @@ -27,69 +27,127 @@ import { sources, } from 'next/dist/compiled/webpack/webpack' -function getModulesIterable(compilation: any, chunk: any) { +function getModuleId(compilation: any, module: any): string | number { if (isWebpack5) { - return compilation.chunkGraph.getChunkModulesIterable(chunk) + return compilation.chunkGraph.getModuleId(module) } - return chunk.modulesIterable + return module.id } -function getModuleId(compilation: any, module: any) { +function getModuleFromDependency( + compilation: any, + dep: any +): webpack.Module & { resource?: string } { if (isWebpack5) { - return compilation.chunkGraph.getModuleId(module) + return compilation.moduleGraph.getModule(dep) } - return module.id + return dep.module +} + +function getOriginModuleFromDependency( + compilation: any, + dep: any +): webpack.Module & { resource?: string } { + if (isWebpack5) { + return compilation.moduleGraph.getParentModule(dep) + } + + return dep.originModule +} + +function getChunkGroupFromBlock( + compilation: any, + block: any +): webpack.compilation.ChunkGroup { + if (isWebpack5) { + return compilation.chunkGraph.getBlockChunkGroup(block) + } + + return block.chunkGroup } function buildManifest( _compiler: webpack.Compiler, compilation: webpack.compilation.Compilation ) { - let manifest: { [k: string]: any[] } = {} - - compilation.chunkGroups.forEach((chunkGroup) => { - if (chunkGroup.isInitial()) { - return - } - - chunkGroup.origins.forEach((chunkGroupOrigin: any) => { - const { request } = chunkGroupOrigin - - chunkGroup.chunks.forEach((chunk: any) => { - chunk.files.forEach((file: string) => { - if ( - !( - (file.endsWith('.js') || file.endsWith('.css')) && - file.match(/^static\/(chunks|css)\//) - ) - ) { - return + let manifest: { [k: string]: { id: string | number; files: string[] } } = {} + + // This is allowed: + // import("./module"); <- ImportDependency + + // We don't support that: + // import(/* webpackMode: "eager" */ "./module") <- ImportEagerDependency + // import(`./module/${param}`) <- ImportContextDependency + + // Find all dependencies blocks which contains a `import()` dependency + const handleBlock = (block: any) => { + block.blocks.forEach(handleBlock) + const chunkGroup = getChunkGroupFromBlock(compilation, block) + for (const dependency of block.dependencies) { + if (dependency.type.startsWith('import()')) { + // get the referenced module + const module = getModuleFromDependency(compilation, dependency) + if (!module) return + + // get the module containing the import() + const originModule = getOriginModuleFromDependency( + compilation, + dependency + ) + const originRequest: string | undefined = originModule?.resource + if (!originRequest) return + + // We construct a "unique" key from origin module and request + // It's not perfect unique, but that will be fine for us. + // We also need to construct the same in the babel plugin. + const key = `${originRequest} -> ${dependency.request}` + + // Capture all files that need to be loaded. + const files = new Set() + + if (manifest[key]) { + // In the "rare" case where multiple chunk groups + // are created for the same `import()` or multiple + // import()s reference the same module, we merge + // the files to make sure to not miss files + // This may cause overfetching in edge cases. + for (const file of manifest[key].files) { + files.add(file) } + } + + // There might not be a chunk group when all modules + // are already loaded. In this case we only need need + // the module id and no files + if (chunkGroup) { + for (const chunk of (chunkGroup as any) + .chunks as webpack.compilation.Chunk[]) { + chunk.files.forEach((file: string) => { + if ( + (file.endsWith('.js') || file.endsWith('.css')) && + file.match(/^static\/(chunks|css)\//) + ) { + files.add(file) + } + }) + } + } - for (const module of getModulesIterable(compilation, chunk)) { - let id = getModuleId(compilation, module) - - if (!manifest[request]) { - manifest[request] = [] - } - - // Avoid duplicate files - if ( - manifest[request].some( - (item) => item.id === id && item.file === file - ) - ) { - continue - } + // usually we have to add the parent chunk groups too + // but we assume that all parents are also imported by + // next/dynamic so they are loaded by the same technique - manifest[request].push({ id, file }) - } - }) - }) - }) - }) + // add the id and files to the manifest + const id = getModuleId(compilation, module) + manifest[key] = { id, files: Array.from(files) } + } + } + } + for (const module of compilation.modules) { + module.blocks.forEach(handleBlock) + } manifest = Object.keys(manifest) .sort() diff --git a/packages/next/next-server/lib/utils.ts b/packages/next/next-server/lib/utils.ts index 228dd892bb20a..ea38a1ddf7e92 100644 --- a/packages/next/next-server/lib/utils.ts +++ b/packages/next/next-server/lib/utils.ts @@ -3,7 +3,6 @@ import { ParsedUrlQuery } from 'querystring' import { ComponentType } from 'react' import { UrlObject } from 'url' import { formatUrl } from './router/utils/format-url' -import { ManifestItem } from '../server/load-components' import { NextRouter } from './router/router' import { Env } from '@next/env' import { BuildManifest } from '../server/get-page-files' @@ -92,7 +91,7 @@ export type NEXT_DATA = { nextExport?: boolean autoExport?: boolean isFallback?: boolean - dynamicIds?: string[] + dynamicIds?: (string | number)[] err?: Error & { statusCode?: number } gsp?: boolean gssp?: boolean @@ -184,7 +183,7 @@ export type DocumentProps = DocumentInitialProps & { inAmpMode: boolean hybridAmp: boolean isDevelopment: boolean - dynamicImports: ManifestItem[] + dynamicImports: string[] assetPrefix?: string canonicalBase: string headTags: any[] diff --git a/packages/next/next-server/server/load-components.ts b/packages/next/next-server/server/load-components.ts index ca99872698306..8422b75ceb475 100644 --- a/packages/next/next-server/server/load-components.ts +++ b/packages/next/next-server/server/load-components.ts @@ -16,11 +16,10 @@ export function interopDefault(mod: any) { export type ManifestItem = { id: number | string - name: string - file: string + files: string[] } -type ReactLoadableManifest = { [moduleId: string]: ManifestItem[] } +type ReactLoadableManifest = { [moduleId: string]: ManifestItem } export type LoadComponentsReturnType = { Component: React.ComponentType diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 07effe84120d4..39a197f9dc08c 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -245,8 +245,8 @@ function renderDocument( ampPath: string inAmpMode: boolean hybridAmp: boolean - dynamicImportsIds: string[] - dynamicImports: ManifestItem[] + dynamicImportsIds: (string | number)[] + dynamicImports: string[] headTags: any isFallback?: boolean gsp?: boolean @@ -1023,21 +1023,20 @@ export async function renderToHTML( throw new Error(message) } - const dynamicImportIdsSet = new Set() - const dynamicImports: ManifestItem[] = [] + const dynamicImportsIds = new Set() + const dynamicImports = new Set() for (const mod of reactLoadableModules) { - const manifestItem: ManifestItem[] = reactLoadableManifest[mod] + const manifestItem: ManifestItem = reactLoadableManifest[mod] if (manifestItem) { - manifestItem.forEach((item) => { - dynamicImports.push(item) - dynamicImportIdsSet.add(item.id as string) + dynamicImportsIds.add(manifestItem.id) + manifestItem.files.forEach((item) => { + dynamicImports.add(item) }) } } - const dynamicImportsIds = [...dynamicImportIdsSet] const hybridAmp = ampState.hybrid const docComponentsRendered: DocumentProps['docComponentsRendered'] = {} @@ -1069,8 +1068,8 @@ export async function renderToHTML( query, inAmpMode, hybridAmp, - dynamicImportsIds, - dynamicImports, + dynamicImportsIds: Array.from(dynamicImportsIds), + dynamicImports: Array.from(dynamicImports), gsp: !!getStaticProps ? true : undefined, gssp: !!getServerSideProps ? true : undefined, gip: hasPageGetInitialProps ? true : undefined, diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 20e521506cc4e..df83949222c9a 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -28,18 +28,6 @@ export type OriginProps = { crossOrigin?: string } -function dedupe(bundles: T[]): T[] { - const files = new Set() - const kept: T[] = [] - - for (const bundle of bundles) { - if (files.has(bundle.file)) continue - files.add(bundle.file) - kept.push(bundle) - } - return kept -} - type DocumentFiles = { sharedFiles: readonly string[] pageFiles: readonly string[] @@ -167,9 +155,9 @@ export class Head extends Component< // Unmanaged files are CSS files that will be handled directly by the // webpack runtime (`mini-css-extract-plugin`). let unmangedFiles: Set = new Set([]) - let dynamicCssFiles = dedupe( - dynamicImports.filter((f) => f.file.endsWith('.css')) - ).map((f) => f.file) + let dynamicCssFiles = Array.from( + new Set(dynamicImports.filter((file) => file.endsWith('.css'))) + ) if (dynamicCssFiles.length) { const existing = new Set(cssFiles) dynamicCssFiles = dynamicCssFiles.filter( @@ -238,18 +226,18 @@ export class Head extends Component< } = this.context return ( - dedupe(dynamicImports) - .map((bundle) => { - if (!bundle.file.endsWith('.js')) { + dynamicImports + .map((file) => { + if (!file.endsWith('.js')) { return null } return ( { devOnlyCacheBusterQueryString, } = this.context - return dedupe(dynamicImports).map((bundle) => { - if (!bundle.file.endsWith('.js') || files.allFiles.includes(bundle.file)) - return null + return dynamicImports.map((file) => { + if (!file.endsWith('.js') || files.allFiles.includes(file)) return null return (