Repository: webpack/webpack (JavaScript)

Issue:
Side-effect-free modules with namespace re-exports are not always elided, causing nondeterministic output
<!-- Please don't delete this template because we'll close your issue -->
<!-- Before creating an issue please make sure you are using the latest version of webpack. -->

# Bug report

Repro: https://github.com/pgoldberg/webpack-namespace-reexport-side-effects-bug

Webpack doesn't always elide side-effect-free modules that only perform namespace re-exports. This results from the `modules` set being unordered, and the `SideEffectsFlagPlugin` not handling it properly. The module doing the re-export must have its incoming connections optimized before the module that's being re-exported, but because the `modules` set is in any order, that isn't always the case.

**What is the expected behavior?**

Side-effect-free modules should always be elided (unless side effect optimization is off or the module is marked as side-effectful in the package.json).

Just opening this issue for tracking, proposed fix is open here: https://github.com/webpack/webpack/pull/17595

**Other relevant information:**
webpack version:  latest
Node.js version: any
Operating System: any



Hints:

Patch:
diff --git a/lib/optimize/SideEffectsFlagPlugin.js b/lib/optimize/SideEffectsFlagPlugin.js
--- a/lib/optimize/SideEffectsFlagPlugin.js
+++ b/lib/optimize/SideEffectsFlagPlugin.js
@@ -243,7 +243,12 @@ class SideEffectsFlagPlugin {
 						);
 
 						logger.time("update dependencies");
-						for (const module of modules) {
+
+						const optimizedModules = new Set();
+
+						const optimizeIncomingConnections = module => {
+							if (optimizedModules.has(module)) return;
+							optimizedModules.add(module);
 							if (module.getSideEffectsConnectionState(moduleGraph) === false) {
 								const exportsInfo = moduleGraph.getExportsInfo(module);
 								for (const connection of moduleGraph.getIncomingConnections(
@@ -258,6 +263,9 @@ class SideEffectsFlagPlugin {
 										(dep instanceof HarmonyImportSpecifierDependency &&
 											!dep.namespaceObjectAsContext)
 									) {
+										if (connection.originModule !== null) {
+											optimizeIncomingConnections(connection.originModule);
+										}
 										// TODO improve for export *
 										if (isReexport && dep.name) {
 											const exportInfo = moduleGraph.getExportInfo(
@@ -314,6 +322,10 @@ class SideEffectsFlagPlugin {
 									}
 								}
 							}
+						};
+
+						for (const module of modules) {
+							optimizeIncomingConnections(module);
 						}
 						logger.timeEnd("update dependencies");
 					}


Test patch:
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/index.js b/test/configCases/side-effects/side-effects-unsorted-modules/index.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/index.js
@@ -0,0 +1,9 @@
+import { b } from "dep";
+
+b.c();
+
+import { modules } from "dep/trackModules.js";
+
+it("should not contain side-effect-free modules", () => {
+	expect(modules).toEqual(["c"]);
+});
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/a.js b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/a.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/a.js
@@ -0,0 +1,3 @@
+import { track } from "./trackModules.js";
+track("a");
+export * as b from "./b.js";
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/b.js b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/b.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/b.js
@@ -0,0 +1,3 @@
+import { track } from "./trackModules.js";
+track("b");
+export * from "./c.js";
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/c.js b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/c.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/c.js
@@ -0,0 +1,3 @@
+import { track } from "./trackModules.js";
+track("c");
+export function c() {}
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/index.js b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/index.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/index.js
@@ -0,0 +1,3 @@
+import { track } from "./trackModules.js";
+track("index");
+export * from "./a.js"
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/package.json b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/package.json
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/package.json
@@ -0,0 +1,6 @@
+{
+  "name": "dep",
+  "version": "1.0.0",
+  "type": "module",
+  "sideEffects": false
+}
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/trackModules.js b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/trackModules.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/node_modules/dep/trackModules.js
@@ -0,0 +1,4 @@
+export const modules = [];
+export function track(name) {
+	modules.push(name);
+}
diff --git a/test/configCases/side-effects/side-effects-unsorted-modules/webpack.config.js b/test/configCases/side-effects/side-effects-unsorted-modules/webpack.config.js
new file mode 100644
--- /dev/null
+++ b/test/configCases/side-effects/side-effects-unsorted-modules/webpack.config.js
@@ -0,0 +1,23 @@
+/** @type {import("../../../../").Configuration} */
+
+class ReorderModulesPlugin {
+	constructor() {}
+
+	apply(compiler) {
+		compiler.hooks.compilation.tap("ReorderModulesPlugin", compilation => {
+			compilation.hooks.seal.tap("ReorderModulesPlugin", () => {
+				const sortedModules = Array.from(compilation.modules).sort((a, _b) =>
+					a.request.includes("b.js") ? -1 : 1
+				);
+				compilation.modules = new Set(sortedModules);
+			});
+		});
+	}
+}
+
+module.exports = {
+	plugins: [new ReorderModulesPlugin()],
+	optimization: {
+		sideEffects: true
+	}
+};