improve react-loadable-plugin (#24281)

Co-authored-by: JJ Kasper <jj@jjsweb.site>



## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added


Closes: #22741

packages/next/build/babel/plugins/react-loadable-plugin.ts

@@ -29,14 +29,19 @@ import {
   types as BabelTypes,
 } from 'next/dist/compiled/babel/core'
 
+import { relative as relativePath } from 'path'
+
 export default function ({
   types: t,
 }: {
   types: typeof BabelTypes
 }): PluginObj {
   return {
     visitor: {
-      ImportDeclaration(path: NodePath<BabelTypes.ImportDeclaration>) {
+      ImportDeclaration(
+        path: NodePath<BabelTypes.ImportDeclaration>,
+        state: any
+      ) {
         let source = path.node.source.value
         if (source !== 'next/dynamic') return
 
@@ -133,14 +138,27 @@ 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) {
               const importArguments = importPath.parentPath.get('arguments')
               if (!Array.isArray(importArguments)) return
               const node: any = importArguments[0].node
               dynamicImports.push(node)
+              dynamicKeys.push(
+                t.binaryExpression(
+                  '+',
+                  t.stringLiteral(
+                    relativePath(
+                      state.file.opts.caller.pagesDir,
+                      state.file.opts.filename
+                    ) + ' -> '
+                  ),
+                  node
+                )
+              )
             },
           })
 
@@ -169,7 +187,7 @@ export default function ({
                 ),
                 t.objectProperty(
                   t.identifier('modules'),
-                  t.arrayExpression(dynamicImports)
+                  t.arrayExpression(dynamicKeys)
                 ),
               ])
             )

packages/next/build/webpack-config.ts

@@ -1141,6 +1141,7 @@ export default async function getBaseWebpackConfig(
       !isServer &&
         new ReactLoadablePlugin({
           filename: REACT_LOADABLE_MANIFEST,
+          pagesDir,
         }),
       !isServer && new DropClientPage(),
       // Moment.js is an extremely popular library that bundles large locale files

packages/next/build/webpack/loaders/next-babel-loader.js

@@ -102,6 +102,7 @@ const customBabelLoader = babelLoader((babel) => {
       options.caller.isServer = isServer
       options.caller.isDev = development
       options.caller.hasJsxRuntime = hasJsxRuntime
+      options.caller.pagesDir = pagesDir
 
       const emitWarning = this.emitWarning.bind(this)
       Object.defineProperty(options.caller, 'onWarning', {

packages/next/build/webpack/plugins/react-loadable-plugin.ts

@@ -27,69 +27,132 @@ import {
   sources,
 } from 'next/dist/compiled/webpack/webpack'
 
-function getModulesIterable(compilation: any, chunk: any) {
+import path from 'path'
+
+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 buildManifest(
-  _compiler: webpack.Compiler,
-  compilation: webpack.compilation.Compilation
-) {
-  let manifest: { [k: string]: any[] } = {}
+function getOriginModuleFromDependency(
+  compilation: any,
+  dep: any
+): webpack.Module & { resource?: string } {
+  if (isWebpack5) {
+    return compilation.moduleGraph.getParentModule(dep)
+  }
 
-  compilation.chunkGroups.forEach((chunkGroup) => {
-    if (chunkGroup.isInitial()) {
-      return
-    }
+  return dep.originModule
+}
 
-    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
-          }
+function getChunkGroupFromBlock(
+  compilation: any,
+  block: any
+): webpack.compilation.ChunkGroup {
+  if (isWebpack5) {
+    return compilation.chunkGraph.getBlockChunkGroup(block)
+  }
 
-          for (const module of getModulesIterable(compilation, chunk)) {
-            let id = getModuleId(compilation, module)
+  return block.chunkGroup
+}
 
-            if (!manifest[request]) {
-              manifest[request] = []
-            }
+function buildManifest(
+  _compiler: webpack.Compiler,
+  compilation: webpack.compilation.Compilation,
+  pagesDir: string
+) {
+  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 = `${path.relative(pagesDir, originRequest)} -> ${
+          dependency.request
+        }`
+
+        // Capture all files that need to be loaded.
+        const files = new Set<string>()
+
+        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)
+              }
+            })
+          }
+        }
 
-            // 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()
@@ -101,13 +164,15 @@ function buildManifest(
 
 export class ReactLoadablePlugin {
   private filename: string
+  private pagesDir: string
 
-  constructor(opts: { filename: string }) {
+  constructor(opts: { filename: string; pagesDir: string }) {
     this.filename = opts.filename
+    this.pagesDir = opts.pagesDir
   }
 
   createAssets(compiler: any, compilation: any, assets: any) {
-    const manifest = buildManifest(compiler, compilation)
+    const manifest = buildManifest(compiler, compilation, this.pagesDir)
     // @ts-ignore: TODO: remove when webpack 5 is stable
     assets[this.filename] = new sources.RawSource(
       JSON.stringify(manifest, null, 2)

packages/next/client/index.tsx

@@ -39,7 +39,7 @@ declare global {
     __NEXT_HYDRATED_CB?: () => void
 
     /* prod */
-    __NEXT_PRELOADREADY?: (ids?: string[]) => void
+    __NEXT_PRELOADREADY?: (ids?: (string | number)[]) => void
     __NEXT_DATA__: NEXT_DATA
     __NEXT_P: any[]
   }

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'
@@ -93,7 +92,7 @@ export type NEXT_DATA = {
   nextExport?: boolean
   autoExport?: boolean
   isFallback?: boolean
-  dynamicIds?: string[]
+  dynamicIds?: (string | number)[]
   err?: Error & { statusCode?: number }
   gsp?: boolean
   gssp?: boolean
@@ -185,7 +184,7 @@ export type DocumentProps = DocumentInitialProps & {
   inAmpMode: boolean
   hybridAmp: boolean
   isDevelopment: boolean
-  dynamicImports: ManifestItem[]
+  dynamicImports: string[]
   assetPrefix?: string
   canonicalBase: string
   headTags: any[]

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

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<string>()
-  const dynamicImports: ManifestItem[] = []
+  const dynamicImportsIds = new Set<string | number>()
+  const dynamicImports = new Set<string>()
 
   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,