From 15b005e536cb8b1dcce65d36bbf3b00602bf7353 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 14 Apr 2023 02:51:25 +0800 Subject: [PATCH 1/5] fix(build): warn dynamic import module with a static import alongside --- packages/vite/src/node/plugins/reporter.ts | 19 ++++++++++++++++++- .../__tests__/dynamic-import.spec.ts | 11 +++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index f36081041ca1c5..3b8990d4eff372 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -111,7 +111,24 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { compressedCount = 0 }, - renderChunk() { + renderChunk(code, chunk) { + for (const id of chunk.moduleIds) { + const module = this.getModuleInfo(id) + if (!module) continue + if (module.importers.length && module.dynamicImporters.length) { + this.warn( + `\n(!) ${ + module.id + } is dynamically imported by ${module.dynamicImporters + .map((m) => m) + .join(', ')} but also statically imported by ${module.importers + .map((m) => m) + .join( + ', ', + )}, dynamic import will not move module into another chunk.\n`, + ) + } + } chunkCount++ if (shouldLogInfo) { if (!tty) { diff --git a/playground/dynamic-import/__tests__/dynamic-import.spec.ts b/playground/dynamic-import/__tests__/dynamic-import.spec.ts index 91e7ae35d0afaf..dbd6adb72f642a 100644 --- a/playground/dynamic-import/__tests__/dynamic-import.spec.ts +++ b/playground/dynamic-import/__tests__/dynamic-import.spec.ts @@ -129,3 +129,14 @@ test('should work with load ../ and contain itself directory', async () => { true, ) }) + +test.runIf(isBuild)( + 'should rollup warn when static and dynamic import a module in same chunk', + async () => { + await untilUpdated( + () => serverLogs.join('\n'), + 'dynamic import will not move module into another chunk', + true, + ) + }, +) From d16e2e247599d8f7d9771654eb03ab006d397f0e Mon Sep 17 00:00:00 2001 From: fi3ework Date: Thu, 18 May 2023 02:56:44 +0800 Subject: [PATCH 2/5] fix: only warn when dynamic importer in same chunk --- packages/vite/src/node/plugins/reporter.ts | 30 ++++++++++++------- .../__tests__/dynamic-import.spec.ts | 14 +++++++-- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index 3b8990d4eff372..bfa6997fc668ea 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -115,20 +115,28 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { for (const id of chunk.moduleIds) { const module = this.getModuleInfo(id) if (!module) continue + // warn if a module is imported both dynamically and statically in a chunk if (module.importers.length && module.dynamicImporters.length) { - this.warn( - `\n(!) ${ - module.id - } is dynamically imported by ${module.dynamicImporters - .map((m) => m) - .join(', ')} but also statically imported by ${module.importers - .map((m) => m) - .join( - ', ', - )}, dynamic import will not move module into another chunk.\n`, - ) + for (const dynamicImporter of module.dynamicImporters) { + if (chunk.moduleIds.includes(dynamicImporter)) { + this.warn( + `\n(!) ${ + module.id + } is dynamically imported by ${module.dynamicImporters + .map((m) => m) + .join( + ', ', + )} but also statically imported by ${module.importers + .map((m) => m) + .join( + ', ', + )}, dynamic import will not move module into another chunk.\n`, + ) + } + } } } + chunkCount++ if (shouldLogInfo) { if (!tty) { diff --git a/playground/dynamic-import/__tests__/dynamic-import.spec.ts b/playground/dynamic-import/__tests__/dynamic-import.spec.ts index dbd6adb72f642a..daa378da6ef1af 100644 --- a/playground/dynamic-import/__tests__/dynamic-import.spec.ts +++ b/playground/dynamic-import/__tests__/dynamic-import.spec.ts @@ -133,10 +133,18 @@ test('should work with load ../ and contain itself directory', async () => { test.runIf(isBuild)( 'should rollup warn when static and dynamic import a module in same chunk', async () => { - await untilUpdated( - () => serverLogs.join('\n'), + const log = serverLogs.join('\n') + expect(log).toContain( 'dynamic import will not move module into another chunk', - true, + ) + expect(log).toMatch( + /\(!\).*\/dynamic-import\/files\/mxd\.js is dynamically imported by/, + ) + expect(log).toMatch( + /\(!\).*\/dynamic-import\/files\/mxd\.json is dynamically imported by/, + ) + expect(log).not.toMatch( + /\(!\).*\/dynamic-import\/nested\/shared\.js is dynamically imported by/, ) }, ) From 8bee3c9c1322d7d11b7d59a058431f0ba0fe0cca Mon Sep 17 00:00:00 2001 From: fi3ework Date: Wed, 7 Jun 2023 22:22:58 +0800 Subject: [PATCH 3/5] chore: add comments --- packages/vite/src/node/plugins/reporter.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index bfa6997fc668ea..dbcfe7cecb218e 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -115,7 +115,8 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { for (const id of chunk.moduleIds) { const module = this.getModuleInfo(id) if (!module) continue - // warn if a module is imported both dynamically and statically in a chunk + // 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) { for (const dynamicImporter of module.dynamicImporters) { if (chunk.moduleIds.includes(dynamicImporter)) { From 31c5ebe093f87c58c66d352491b04a4dafac3292 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Fri, 9 Jun 2023 22:57:59 +0800 Subject: [PATCH 4/5] chore: comment --- packages/vite/src/node/plugins/reporter.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index dbcfe7cecb218e..d742effdc940a1 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -119,6 +119,10 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin { // warn that the dynamic imported module will not be moved to another chunk (#12850). if (module.importers.length && module.dynamicImporters.length) { for (const dynamicImporter of module.dynamicImporters) { + // 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 (chunk.moduleIds.includes(dynamicImporter)) { this.warn( `\n(!) ${ From fe089c7e15cd07cc91755e6c2de6a7d235cecd6b Mon Sep 17 00:00:00 2001 From: fi3ework Date: Sat, 10 Jun 2023 16:16:26 +0800 Subject: [PATCH 5/5] chore: clean code --- packages/vite/src/node/plugins/reporter.ts | 38 ++++++++++------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/packages/vite/src/node/plugins/reporter.ts b/packages/vite/src/node/plugins/reporter.ts index d742effdc940a1..0135f552e8845d 100644 --- a/packages/vite/src/node/plugins/reporter.ts +++ b/packages/vite/src/node/plugins/reporter.ts @@ -118,26 +118,24 @@ 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 (module.importers.length && module.dynamicImporters.length) { - for (const dynamicImporter of module.dynamicImporters) { - // 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 (chunk.moduleIds.includes(dynamicImporter)) { - this.warn( - `\n(!) ${ - module.id - } is dynamically imported by ${module.dynamicImporters - .map((m) => m) - .join( - ', ', - )} but also statically imported by ${module.importers - .map((m) => m) - .join( - ', ', - )}, dynamic import will not move module into another chunk.\n`, - ) - } + // 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)) + ) { + this.warn( + `\n(!) ${ + module.id + } is dynamically imported by ${module.dynamicImporters + .map((m) => m) + .join(', ')} but also statically imported by ${module.importers + .map((m) => m) + .join( + ', ', + )}, dynamic import will not move module into another chunk.\n`, + ) } } }