From 5168e11b2141774b45826ddbe10fea9a1f972002 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 5 Feb 2020 06:54:57 +0100 Subject: [PATCH] Improve coverage --- src/Chunk.ts | 11 +++++++---- src/Module.ts | 7 +------ src/utils/chunkAssignment.ts | 8 +++----- src/utils/executionOrder.ts | 3 --- .../avoid-imports-preserve-modules/_config.js | 7 +++++++ .../_expected/amd/main1.js | 9 +++++++++ .../_expected/amd/main2.js | 5 +++++ .../_expected/cjs/main1.js | 9 +++++++++ .../_expected/cjs/main2.js | 5 +++++ .../_expected/es/main1.js | 7 +++++++ .../_expected/es/main2.js | 3 +++ .../_expected/system/main1.js | 15 +++++++++++++++ .../_expected/system/main2.js | 11 +++++++++++ .../avoid-imports-preserve-modules/a.js | 1 + .../avoid-imports-preserve-modules/b.js | 1 + .../avoid-imports-preserve-modules/c.js | 1 + .../avoid-imports-preserve-modules/index.js | 3 +++ .../avoid-imports-preserve-modules/main.js | 3 +++ .../avoid-imports-preserve-modules/one.js | 2 ++ 19 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_config.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main1.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main2.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main1.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main2.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main1.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main2.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main1.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main2.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/a.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/b.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/c.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/index.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/main.js create mode 100644 test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/one.js diff --git a/src/Chunk.ts b/src/Chunk.ts index 630d3134c92..eefa02ff532 100644 --- a/src/Chunk.ts +++ b/src/Chunk.ts @@ -549,7 +549,7 @@ export default class Chunk { this.facadeModule !== null ) { for (const dep of this.dependencies) { - if (dep instanceof Chunk) this.inlineChunkDependencies(dep, true); + if (dep instanceof Chunk) this.inlineChunkDependencies(dep); } } const sortedDependencies = [...this.dependencies]; @@ -1043,14 +1043,17 @@ export default class Chunk { return relativePath.startsWith('../') ? relativePath : './' + relativePath; } - private inlineChunkDependencies(chunk: Chunk, deep: boolean) { + private inlineChunkDependencies(chunk: Chunk) { for (const dep of chunk.dependencies) { if (dep instanceof ExternalModule) { this.dependencies.add(dep); } else { - if (dep === this) continue; + // At the moment, circular dependencies between chunks are not possible; this will + // change if we ever add logic to ensure correct execution order or open up the + // chunking to plugins + // if (dep === this) continue; this.dependencies.add(dep); - if (deep) this.inlineChunkDependencies(dep, true); + this.inlineChunkDependencies(dep); } } } diff --git a/src/Module.ts b/src/Module.ts index 62d49e835f0..83501d71bc0 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -587,12 +587,7 @@ export default class Module { linkDependencies() { for (const source of this.sources) { - const id = this.resolvedIds[source].id; - - if (id) { - const module = this.graph.moduleById.get(id)!; - this.dependencies.add(module); - } + this.dependencies.add(this.graph.moduleById.get(this.resolvedIds[source].id)!); } for (const { resolution } of this.dynamicImports) { if (resolution instanceof Module || resolution instanceof ExternalModule) { diff --git a/src/utils/chunkAssignment.ts b/src/utils/chunkAssignment.ts index 53d5f2c41b6..c0eaff596ab 100644 --- a/src/utils/chunkAssignment.ts +++ b/src/utils/chunkAssignment.ts @@ -65,11 +65,9 @@ export function getChunkAssignments( return true; } - if (manualChunkModules) { - for (const chunkName of Object.keys(manualChunkModules)) { - for (const entry of manualChunkModules[chunkName]) { - assignEntryToStaticDependencies(entry, null, chunkName); - } + for (const chunkName of Object.keys(manualChunkModules)) { + for (const entry of manualChunkModules[chunkName]) { + assignEntryToStaticDependencies(entry, null, chunkName); } } diff --git a/src/utils/executionOrder.ts b/src/utils/executionOrder.ts index 4f8997174f1..4ef4e7fde39 100644 --- a/src/utils/executionOrder.ts +++ b/src/utils/executionOrder.ts @@ -22,8 +22,6 @@ export function analyseModuleExecution(entryModules: Module[]) { const orderedModules: Module[] = []; const analyseModule = (module: Module | ExternalModule) => { - if (analysedModules.has(module)) return; - if (module instanceof Module) { for (const dependency of module.dependencies) { if (parents.has(dependency)) { @@ -74,7 +72,6 @@ function getCyclePath( while (nextModule !== module) { path.push(relativeId(nextModule.id)); nextModule = parents.get(nextModule)!; - if (!nextModule) break; } path.push(path[0]); path.reverse(); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_config.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_config.js new file mode 100644 index 00000000000..e3fdb18ef33 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_config.js @@ -0,0 +1,7 @@ +module.exports = { + description: + 'avoids empty imports if they do not have side-effects when preserving modules (#3359)', + options: { + preserveModules: true + } +}; diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main1.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main1.js new file mode 100644 index 00000000000..8acd4520991 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main1.js @@ -0,0 +1,9 @@ +define(['external-side-effect'], function (externalSideEffect) { 'use strict'; + + function onlyUsedByOne() { + console.log('Hello'); + } + + onlyUsedByOne(); + +}); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main2.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main2.js new file mode 100644 index 00000000000..52c48560358 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/amd/main2.js @@ -0,0 +1,5 @@ +define(['external-side-effect'], function (externalSideEffect) { 'use strict'; + + console.log('main2'); + +}); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main1.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main1.js new file mode 100644 index 00000000000..5fa6a606ea6 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main1.js @@ -0,0 +1,9 @@ +'use strict'; + +require('external-side-effect'); + +function onlyUsedByOne() { + console.log('Hello'); +} + +onlyUsedByOne(); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main2.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main2.js new file mode 100644 index 00000000000..6870306ded8 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/cjs/main2.js @@ -0,0 +1,5 @@ +'use strict'; + +require('external-side-effect'); + +console.log('main2'); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main1.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main1.js new file mode 100644 index 00000000000..1e5e5a759b2 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main1.js @@ -0,0 +1,7 @@ +import 'external-side-effect'; + +function onlyUsedByOne() { + console.log('Hello'); +} + +onlyUsedByOne(); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main2.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main2.js new file mode 100644 index 00000000000..7d885833b90 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/es/main2.js @@ -0,0 +1,3 @@ +import 'external-side-effect'; + +console.log('main2'); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main1.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main1.js new file mode 100644 index 00000000000..184dec65313 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main1.js @@ -0,0 +1,15 @@ +System.register(['external-side-effect'], function () { + 'use strict'; + return { + setters: [function () {}], + execute: function () { + + function onlyUsedByOne() { + console.log('Hello'); + } + + onlyUsedByOne(); + + } + }; +}); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main2.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main2.js new file mode 100644 index 00000000000..978a1d82672 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/_expected/system/main2.js @@ -0,0 +1,11 @@ +System.register(['external-side-effect'], function () { + 'use strict'; + return { + setters: [function () {}], + execute: function () { + + console.log('main2'); + + } + }; +}); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/a.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/a.js new file mode 100644 index 00000000000..cc798ff50da --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/a.js @@ -0,0 +1 @@ +export const a = 1; diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/b.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/b.js new file mode 100644 index 00000000000..202103085ce --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/b.js @@ -0,0 +1 @@ +export const b = 2; diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/c.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/c.js new file mode 100644 index 00000000000..5f0cabef84f --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/c.js @@ -0,0 +1 @@ +export const c = 3; diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/index.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/index.js new file mode 100644 index 00000000000..c2357812df9 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/index.js @@ -0,0 +1,3 @@ +export { a } from './a'; +export { b } from './b'; +export { c } from './c'; diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/main.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/main.js new file mode 100644 index 00000000000..6279273d11d --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/main.js @@ -0,0 +1,3 @@ +import { a } from './index'; +import { d } from './one'; +console.log(a + d); diff --git a/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/one.js b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/one.js new file mode 100644 index 00000000000..8a9f8689a62 --- /dev/null +++ b/test/chunking-form/samples/side-effect-free-dependencies/avoid-imports-preserve-modules/one.js @@ -0,0 +1,2 @@ +import { b } from './index'; +export const d = b + 4;