From e0f49a7f8dd1c45ad9998487926a0ab94236a383 Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Tue, 18 Jul 2023 00:24:02 +0900 Subject: [PATCH 1/8] feat(node): remove warning about ineffective dynamic import of node_modules --- packages/vite/src/node/plugins/reporter.ts | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index 0135f552e8845d..d031caec90a650 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -115,16 +115,29 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { for (const id of chunk.moduleIds) { const module = this.getModuleInfo(id) if (!module) continue + + const hasStaticImport = module.importers.length > 0 + const hasDynamicImport = module.dynamicImporters.length > 0 + // When a dynamic importer shares a chunk with the imported module, // warn that the dynamic imported module will not be moved to another chunk (#12850). - if (module.importers.length && module.dynamicImporters.length) { + if (hasStaticImport && hasDynamicImport) { + // Check if the module is imported by an external dependency, because + // warning ineffective dynamic import of an external dependency is not helpful. + const isExternalDependency = (module: string) => + module.includes('/node_modules/') + const isInTheSameChunk = (module: string) => + chunk.moduleIds.includes(module) + const detectedDirectIneffectiveDynamicImport = + module.dynamicImporters.some( + (m) => !isExternalDependency(m) && isInTheSameChunk(m), + ) + // Filter out the intersection of dynamic importers and sibling modules in // the same chunk. The intersecting dynamic importers' dynamic import is not // expected to work. Note we're only detecting the direct ineffective // dynamic import here. - if ( - module.dynamicImporters.some((m) => chunk.moduleIds.includes(m)) - ) { + if (detectedDirectIneffectiveDynamicImport) { this.warn( `\n(!) ${ module.id From 135e9d618944157de6b5314efd3c62d33ced2db0 Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Tue, 18 Jul 2023 12:57:28 +0900 Subject: [PATCH 2/8] feat(node): add config option of `warnExternalChunkRender` --- packages/vite/src/node/build.ts | 7 +++++++ packages/vite/src/node/config.ts | 2 +- packages/vite/src/node/plugins/reporter.ts | 22 +++++++++++----------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index dfaab8362784c5..b9bf8993555ef4 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -244,6 +244,12 @@ export interface BuildOptions { * @default null */ watch?: WatcherOptions | null + /** + * Whether to log warnings emitted by ineffective dynamic imports. + * Ineffective dynamic imports are imports that do not split chunks (#12850). + * @default false + */ + warnExternalChunkRender: boolean } export interface LibraryOptions { @@ -350,6 +356,7 @@ export function resolveBuildOptions( reportCompressedSize: true, chunkSizeWarningLimit: 500, watch: null, + warnExternalChunkRender: false, } const userBuildOptions = raw diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 8c83bf787896ea..74353f7fd09d87 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -482,7 +482,7 @@ export async function resolveConfig( optimizeDeps: { disabled: false }, ssr: { optimizeDeps: { disabled: false } }, }) - config.build ??= {} + config.build ??= {} as BuildOptions config.build.commonjsOptions = { include: [] } } diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index d031caec90a650..a50c56ce503fe7 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -122,22 +122,22 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { // When a dynamic importer shares a chunk with the imported module, // warn that the dynamic imported module will not be moved to another chunk (#12850). if (hasStaticImport && hasDynamicImport) { - // Check if the module is imported by an external dependency, because - // warning ineffective dynamic import of an external dependency is not helpful. - const isExternalDependency = (module: string) => - module.includes('/node_modules/') - const isInTheSameChunk = (module: string) => - chunk.moduleIds.includes(module) - const detectedDirectIneffectiveDynamicImport = - module.dynamicImporters.some( - (m) => !isExternalDependency(m) && isInTheSameChunk(m), - ) + const warningCondition = (module: string) => { + const isExternalDependency = module.includes('/node_modules/') + const isInSameChunk = chunk.moduleIds.includes(module) + if (!isInSameChunk) return false + if (!isExternalDependency) return true + return config.build.warnExternalChunkRender + } + + const detectedIneffectiveDynamicImport = + module.dynamicImporters.some(warningCondition) // Filter out the intersection of dynamic importers and sibling modules in // the same chunk. The intersecting dynamic importers' dynamic import is not // expected to work. Note we're only detecting the direct ineffective // dynamic import here. - if (detectedDirectIneffectiveDynamicImport) { + if (detectedIneffectiveDynamicImport) { this.warn( `\n(!) ${ module.id From 4b9ffeb3d3b2e7c7ae59571d4909e8c183330d63 Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Tue, 18 Jul 2023 13:53:11 +0900 Subject: [PATCH 3/8] fix(node): optionalize `warnExternalChunkRender` --- packages/vite/src/node/build.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index b9bf8993555ef4..8579ffd25248ab 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -249,7 +249,7 @@ export interface BuildOptions { * Ineffective dynamic imports are imports that do not split chunks (#12850). * @default false */ - warnExternalChunkRender: boolean + warnExternalChunkRender?: boolean } export interface LibraryOptions { From 33bca87ff40cc5aaad7dec854c569be5ee2a340d Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Thu, 20 Jul 2023 14:27:25 +0900 Subject: [PATCH 4/8] docs(config): add information about `build.warnExternalChunkRender` --- docs/config/build-options.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/config/build-options.md b/docs/config/build-options.md index b5872e363de973..a20cc14730278a 100644 --- a/docs/config/build-options.md +++ b/docs/config/build-options.md @@ -250,4 +250,13 @@ Set to `{}` to enable rollup watcher. This is mostly used in cases that involve There are cases that file system watching does not work with WSL2. See [`server.watch`](./server-options.md#server-watch) for more details. +## build.warnExternalChunkRender + +- **Type:** `boolean` +- **Default:** `false` + +Vite, by default, tries to split dynamic imports into separate chunks. However, when a dynamic import is both statically and dynamically imported in the same module, it can't be split into a separate chunk. We refer to this situation as an _Ineffective Dynamic Import_, and Vite issues a warning about it. + +This warning can be informative for an app's own module, but it can also be noisy when used with third-party libraries. Therefore, Vite skips the warning of ineffective dynamic imports produced by modules that exist in `node_modules`. Setting this option to `true` will enable the warning for all ineffective dynamic imports. + ::: From 26a49ef63186f4be266255de277c0195125167c0 Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Thu, 20 Jul 2023 16:04:55 +0900 Subject: [PATCH 5/8] fix(node): remove config option `warnExternalChunkRender` --- docs/config/build-options.md | 9 --------- packages/vite/src/node/build.ts | 7 ------- packages/vite/src/node/plugins/reporter.ts | 3 +-- 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/docs/config/build-options.md b/docs/config/build-options.md index a20cc14730278a..b5872e363de973 100644 --- a/docs/config/build-options.md +++ b/docs/config/build-options.md @@ -250,13 +250,4 @@ Set to `{}` to enable rollup watcher. This is mostly used in cases that involve There are cases that file system watching does not work with WSL2. See [`server.watch`](./server-options.md#server-watch) for more details. -## build.warnExternalChunkRender - -- **Type:** `boolean` -- **Default:** `false` - -Vite, by default, tries to split dynamic imports into separate chunks. However, when a dynamic import is both statically and dynamically imported in the same module, it can't be split into a separate chunk. We refer to this situation as an _Ineffective Dynamic Import_, and Vite issues a warning about it. - -This warning can be informative for an app's own module, but it can also be noisy when used with third-party libraries. Therefore, Vite skips the warning of ineffective dynamic imports produced by modules that exist in `node_modules`. Setting this option to `true` will enable the warning for all ineffective dynamic imports. - ::: diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index 8579ffd25248ab..dfaab8362784c5 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -244,12 +244,6 @@ export interface BuildOptions { * @default null */ watch?: WatcherOptions | null - /** - * Whether to log warnings emitted by ineffective dynamic imports. - * Ineffective dynamic imports are imports that do not split chunks (#12850). - * @default false - */ - warnExternalChunkRender?: boolean } export interface LibraryOptions { @@ -356,7 +350,6 @@ export function resolveBuildOptions( reportCompressedSize: true, chunkSizeWarningLimit: 500, watch: null, - warnExternalChunkRender: false, } const userBuildOptions = raw diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index a50c56ce503fe7..d6b48d4ed5defc 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -126,8 +126,7 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { const isExternalDependency = module.includes('/node_modules/') const isInSameChunk = chunk.moduleIds.includes(module) if (!isInSameChunk) return false - if (!isExternalDependency) return true - return config.build.warnExternalChunkRender + return !isExternalDependency } const detectedIneffectiveDynamicImport = From c62b9daeae3c421e6c8e2d523fdc28e00d90f205 Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Thu, 20 Jul 2023 16:07:14 +0900 Subject: [PATCH 6/8] fix(node): conform with the conventions --- packages/vite/src/node/plugins/reporter.ts | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index d6b48d4ed5defc..5c7865686b79b0 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -4,7 +4,7 @@ import { promisify } from 'node:util' import colors from 'picocolors' import type { Plugin } from 'rollup' import type { ResolvedConfig } from '../config' -import { isDefined, normalizePath } from '../utils' +import { isDefined, isInNodeModules, normalizePath } from '../utils' import { LogLevels } from '../logger' const groups = [ @@ -115,27 +115,16 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { for (const id of chunk.moduleIds) { const module = this.getModuleInfo(id) if (!module) continue - - const hasStaticImport = module.importers.length > 0 - const hasDynamicImport = module.dynamicImporters.length > 0 - // When a dynamic importer shares a chunk with the imported module, // warn that the dynamic imported module will not be moved to another chunk (#12850). - if (hasStaticImport && hasDynamicImport) { - const warningCondition = (module: string) => { - const isExternalDependency = module.includes('/node_modules/') - const isInSameChunk = chunk.moduleIds.includes(module) - if (!isInSameChunk) return false - return !isExternalDependency - } - - const detectedIneffectiveDynamicImport = - module.dynamicImporters.some(warningCondition) - + if (module.importers.length > 0 && module.dynamicImporters.length > 0) { // Filter out the intersection of dynamic importers and sibling modules in // the same chunk. The intersecting dynamic importers' dynamic import is not // expected to work. Note we're only detecting the direct ineffective // dynamic import here. + const detectedIneffectiveDynamicImport = module.dynamicImporters.some( + (id) => !isInNodeModules(id) && chunk.moduleIds.includes(id), + ) if (detectedIneffectiveDynamicImport) { this.warn( `\n(!) ${ From 3e8736d6e9fa8a0ad4b0546e87d019ee1784cc2f Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Thu, 20 Jul 2023 16:17:07 +0900 Subject: [PATCH 7/8] fix(node): remove unnecessary type assertion --- packages/vite/src/node/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 74353f7fd09d87..8c83bf787896ea 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -482,7 +482,7 @@ export async function resolveConfig( optimizeDeps: { disabled: false }, ssr: { optimizeDeps: { disabled: false } }, }) - config.build ??= {} as BuildOptions + config.build ??= {} config.build.commonjsOptions = { include: [] } } From 7dc2085c427addcd76ed94d42d2d4c1fd25df467 Mon Sep 17 00:00:00 2001 From: Chaejun Lee Date: Thu, 20 Jul 2023 16:18:37 +0900 Subject: [PATCH 8/8] fix(node): change back conditional to how it was --- packages/vite/src/node/plugins/reporter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index 5c7865686b79b0..2584ab5db3e654 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -117,7 +117,7 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { if (!module) continue // When a dynamic importer shares a chunk with the imported module, // warn that the dynamic imported module will not be moved to another chunk (#12850). - if (module.importers.length > 0 && module.dynamicImporters.length > 0) { + if (module.importers.length && module.dynamicImporters.length) { // Filter out the intersection of dynamic importers and sibling modules in // the same chunk. The intersecting dynamic importers' dynamic import is not // expected to work. Note we're only detecting the direct ineffective