Merge pull request #7430 from webpack/bugfix/side-effects-reasons

fixup reasons when redirecting dependencies for side-effects

lib/Stats.js

@@ -513,6 +513,7 @@ class Stats {
 								? reason.module.readableIdentifier(requestShortener)
 								: null,
 							type: reason.dependency ? reason.dependency.type : null,
+							explanation: reason.explanation,
 							userRequest: reason.dependency
 								? reason.dependency.userRequest
 								: null
@@ -1058,6 +1059,10 @@ class Stats {
 						colors.normal(" ");
 						colors.normal(reason.loc);
 					}
+					if (reason.explanation) {
+						colors.normal(" ");
+						colors.cyan(reason.explanation);
+					}
 					newline();
 				}
 			}

lib/optimize/ModuleConcatenationPlugin.js

@@ -313,7 +313,7 @@ class ModuleConcatenationPlugin {
 				module.dependencies
 
 					// Only harmony Dependencies
-					.filter(dep => dep instanceof HarmonyImportDependency && dep.module)
+					.filter(dep => dep instanceof HarmonyImportDependency)
 
 					// Get reference info for this dependency
 					.map(dep => dep.getReference())

lib/optimize/SideEffectsFlagPlugin.js

@@ -105,7 +105,8 @@ class SideEffectsFlagPlugin {
 					for (const pair of reexportMaps) {
 						const module = pair[0];
 						const map = pair[1];
-						for (const reason of module.reasons) {
+						for (let i = 0; i < module.reasons.length; i++) {
+							const reason = module.reasons[i];
 							const dep = reason.dependency;
 							if (
 								dep instanceof HarmonyImportSpecifierDependency &&
@@ -115,6 +116,15 @@ class SideEffectsFlagPlugin {
 								if (mapping) {
 									dep.redirectedModule = mapping.module;
 									dep.redirectedId = mapping.exportName;
+									module.reasons.splice(i--, 1);
+									mapping.module.addReason(
+										reason.module,
+										dep,
+										reason.explanation
+											? reason.explanation +
+											  " (skipped side-effect-free modules)"
+											: "(skipped side-effect-free modules)"
+									);
 								}
 							}
 						}

test/__snapshots__/StatsTestCases.test.js.snap

@@ -2062,6 +2062,55 @@ Child
         ModuleConcatenation bailout: Module is referenced from these modules with unsupported syntax: ./first.js (referenced with import())"
 `;
 
+exports[`StatsTestCases should print correct stats for side-effects-issue-7428 1`] = `
+"Hash: 9d82ff1a0201e9c2ffa4
+Time: Xms
+Built at: Thu Jan 01 1970 00:00:00 GMT
+  Asset       Size  Chunks             Chunk Names
+   0.js  481 bytes       0  [emitted]  
+main.js   8.31 KiB       1  [emitted]  main
+Entrypoint main = main.js
+[0] ./components/src/CompAB/utils.js 97 bytes {1} [built]
+    harmony side effect evaluation ./utils [1] ./main.js + 1 modules 1:0-30
+    harmony import specifier ./utils [1] ./main.js + 1 modules 5:2-5
+    harmony side effect evaluation ./utils [4] ./components/src/CompAB/CompA.js 1:0-35
+    harmony import specifier ./utils [4] ./components/src/CompAB/CompA.js 5:5-12
+[1] ./main.js + 1 modules 231 bytes {1} [built]
+    single entry ./main.js  main
+    | ./main.js 144 bytes [built]
+    |     single entry ./main.js  main
+    | ./components/src/CompAB/CompB.js 77 bytes [built]
+    |     [only some exports used: default]
+    |     harmony import specifier ./components  ./main.js 4:15-20 (skipped side-effect-free modules)
+    |     harmony side effect evaluation ./CompB [7] ./components/src/CompAB/index.js 2:0-43
+    |     harmony export imported specifier ./CompB [7] ./components/src/CompAB/index.js 2:0-43
+[2] ./components/src/index.js 84 bytes [built]
+    [no exports used]
+    harmony side effect evaluation ./components [1] ./main.js + 1 modules 1:0-44
+    harmony side effect evaluation ./components [3] ./foo.js 1:0-37
+[3] ./foo.js 101 bytes {0} [built]
+    import() ./foo  ./main.js 6:0-15
+[4] ./components/src/CompAB/CompA.js 89 bytes {1} [built]
+    [only some exports used: default]
+    harmony import specifier ./components  ./main.js 3:15-20 (skipped side-effect-free modules)
+    harmony import specifier ./components [3] ./foo.js 3:20-25 (skipped side-effect-free modules)
+    harmony side effect evaluation ./CompA [7] ./components/src/CompAB/index.js 1:0-43
+    harmony export imported specifier ./CompA [7] ./components/src/CompAB/index.js 1:0-43
+[5] ./components/src/CompC/CompC.js 33 bytes [built]
+    [no exports used]
+    harmony side effect evaluation ./CompC [6] ./components/src/CompC/index.js 1:0-34
+    harmony export imported specifier ./CompC [6] ./components/src/CompC/index.js 1:0-34
+[6] ./components/src/CompC/index.js 34 bytes [built]
+    [no exports used]
+    harmony side effect evaluation ./CompC [2] ./components/src/index.js 2:0-43
+    harmony export imported specifier ./CompC [2] ./components/src/index.js 2:0-43
+[7] ./components/src/CompAB/index.js 87 bytes [built]
+    [no exports used]
+    harmony side effect evaluation ./CompAB [2] ./components/src/index.js 1:0-40
+    harmony export imported specifier ./CompAB [2] ./components/src/index.js 1:0-40
+    harmony export imported specifier ./CompAB [2] ./components/src/index.js 1:0-40"
+`;
+
 exports[`StatsTestCases should print correct stats for side-effects-simple-unused 1`] = `
 "Hash: 0cb74fac5f3c1b7f2150
 Time: Xms

test/statsCases/side-effects-issue-7428/components/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "components",
+  "version": "1.0.0",
+  "main": "src/index.js",
+  "sideEffects": false
+}

test/statsCases/side-effects-issue-7428/components/src/CompAB/CompA.js

@@ -0,0 +1,6 @@
+import * as methods from './utils';
+
+export default { 
+  name: 'CompA', 
+  ...methods,
+};

test/statsCases/side-effects-issue-7428/components/src/CompAB/CompB.js

@@ -0,0 +1,6 @@
+import { fnB } from './utils';
+
+export default { 
+  name: 'CompB', 
+  fnB,
+};

test/statsCases/side-effects-issue-7428/components/src/CompAB/index.js

@@ -0,0 +1,2 @@
+export { default as CompA } from './CompA';
+export { default as CompB } from './CompB';

test/statsCases/side-effects-issue-7428/components/src/CompAB/utils.js

@@ -0,0 +1,2 @@
+export const fnA = () => { console.log('fnA') };
+export const fnB = () => { console.log('fnB') };

test/statsCases/side-effects-issue-7428/components/src/CompC/CompC.js

@@ -0,0 +1 @@
+export default { name: 'CompC' };

test/statsCases/side-effects-issue-7428/components/src/CompC/index.js

@@ -0,0 +1 @@
+export { default } from './CompC';

test/statsCases/side-effects-issue-7428/components/src/index.js

@@ -0,0 +1,2 @@
+export { CompA, CompB } from './CompAB';
+export { default as CompC } from './CompC';

test/statsCases/side-effects-issue-7428/foo.js

@@ -0,0 +1,3 @@
+import { CompA } from './components';
+
+export default { ...CompA, fnB: () => { console.log('hi') } }

test/statsCases/side-effects-issue-7428/main.js

@@ -0,0 +1,8 @@
+import { CompA, CompB } from './components';
+
+window.CompA = CompA;
+window.CompB = CompB;
+
+import('./foo').then((m) => {
+  m.default.fnB();
+});

test/statsCases/side-effects-issue-7428/webpack.config.js

@@ -0,0 +1,14 @@
+module.exports = {
+	mode: "none",
+	entry: "./main.js",
+	optimization: {
+		usedExports: true,
+		sideEffects: true,
+		concatenateModules: true
+	},
+	stats: {
+		nestedModules: true,
+		usedExports: true,
+		reasons: true
+	}
+};