Observe side-effects in files containing a default export declaration…

… that reexports a variable (#3572)

* Observe side-effects in files containing a default export declaration that reexports a variable

* Fiddle with blinking test

src/Chunk.ts

@@ -129,7 +129,11 @@ export default class Chunk {
 		for (const dependency of facadedModule.getDependenciesToBeIncluded()) {
 			chunk.dependencies.add(dependency instanceof Module ? dependency.chunk! : dependency);
 		}
-		if (!chunk.dependencies.has(facadedModule.chunk!) && facadedModule.hasEffects()) {
+		if (
+			!chunk.dependencies.has(facadedModule.chunk!) &&
+			facadedModule.moduleSideEffects &&
+			facadedModule.hasEffects()
+		) {
 			chunk.dependencies.add(facadedModule.chunk!);
 		}
 		chunk.facadeModule = facadedModule;

src/Module.ts

@@ -341,33 +341,39 @@ export default class Module {
 	getDependenciesToBeIncluded(): Set<Module | ExternalModule> {
 		if (this.relevantDependencies) return this.relevantDependencies;
 		const relevantDependencies = new Set<Module | ExternalModule>();
-		for (let variable of this.imports) {
-			if (variable instanceof SyntheticNamedExportVariable) {
-				variable = variable.getBaseVariable();
-			} else if (variable instanceof ExportDefaultVariable) {
-				variable = variable.getOriginalVariable();
-			}
-			relevantDependencies.add(variable.module!);
-		}
+		const additionalSideEffectModules = new Set();
+		let dependencyVariables = this.imports;
 		if (
 			this.isEntryPoint ||
 			this.includedDynamicImporters.length > 0 ||
 			this.graph.preserveModules
 		) {
+			dependencyVariables = new Set(dependencyVariables);
 			for (const exportName of [...this.getReexports(), ...this.getExports()]) {
-				let variable = this.getVariableForExportName(exportName);
-				if (variable instanceof SyntheticNamedExportVariable) {
-					variable = variable.getBaseVariable();
-				} else if (variable instanceof ExportDefaultVariable) {
-					variable = variable.getOriginalVariable();
+				dependencyVariables.add(this.getVariableForExportName(exportName));
+			}
+		}
+		for (let variable of dependencyVariables) {
+			if (variable instanceof SyntheticNamedExportVariable) {
+				variable = variable.getBaseVariable();
+			} else if (variable instanceof ExportDefaultVariable) {
+				const { modules, original } = variable.getOriginalVariableAndDeclarationModules();
+				variable = original;
+				for (const module of modules) {
+					additionalSideEffectModules.add(module);
 				}
-				relevantDependencies.add(variable.module!);
 			}
+			relevantDependencies.add(variable.module!);
 		}
 		if (this.graph.treeshakingOptions) {
 			const possibleDependencies = new Set(this.dependencies);
 			for (const dependency of possibleDependencies) {
-				if (!dependency.moduleSideEffects || relevantDependencies.has(dependency)) continue;
+				if (
+					!(dependency.moduleSideEffects || additionalSideEffectModules.has(dependency)) ||
+					relevantDependencies.has(dependency)
+				) {
+					continue;
+				}
 				if (dependency instanceof ExternalModule || dependency.hasEffects()) {
 					relevantDependencies.add(dependency);
 				} else {
@@ -530,9 +536,7 @@ export default class Module {
 	}
 
 	hasEffects() {
-		return (
-			this.moduleSideEffects && this.ast.included && this.ast.hasEffects(createHasEffectsContext())
-		);
+		return this.ast.included && this.ast.hasEffects(createHasEffectsContext());
 	}
 
 	include(): void {

src/ast/variables/ExportDefaultVariable.ts

@@ -1,4 +1,4 @@
-import { AstContext } from '../../Module';
+import Module, { AstContext } from '../../Module';
 import ClassDeclaration from '../nodes/ClassDeclaration';
 import ExportDefaultDeclaration from '../nodes/ExportDefaultDeclaration';
 import FunctionDeclaration from '../nodes/FunctionDeclaration';
@@ -12,7 +12,10 @@ export default class ExportDefaultVariable extends LocalVariable {
 
 	// Not initialised during construction
 	private originalId: IdentifierWithVariable | null = null;
-	private originalVariable: Variable | null = null;
+	private originalVariableAndDeclarationModules: {
+		modules: Module[];
+		original: Variable;
+	} | null = null;
 
 	constructor(
 		name: string,
@@ -61,22 +64,34 @@ export default class ExportDefaultVariable extends LocalVariable {
 	}
 
 	getOriginalVariable(): Variable {
-		if (this.originalVariable === null) {
+		return this.getOriginalVariableAndDeclarationModules().original;
+	}
+
+	getOriginalVariableAndDeclarationModules(): { modules: Module[]; original: Variable } {
+		if (this.originalVariableAndDeclarationModules === null) {
 			if (
 				!this.originalId ||
 				(!this.hasId &&
 					(this.originalId.variable.isReassigned ||
 						this.originalId.variable instanceof UndefinedVariable))
 			) {
-				this.originalVariable = this;
+				this.originalVariableAndDeclarationModules = { modules: [], original: this };
 			} else {
 				const assignedOriginal = this.originalId.variable;
-				this.originalVariable =
-					assignedOriginal instanceof ExportDefaultVariable
-						? assignedOriginal.getOriginalVariable()
-						: assignedOriginal;
+				if (assignedOriginal instanceof ExportDefaultVariable) {
+					const { modules, original } = assignedOriginal.getOriginalVariableAndDeclarationModules();
+					this.originalVariableAndDeclarationModules = {
+						modules: modules.concat(this.module),
+						original
+					};
+				} else {
+					this.originalVariableAndDeclarationModules = {
+						modules: [this.module],
+						original: assignedOriginal
+					};
+				}
 			}
 		}
-		return this.originalVariable;
+		return this.originalVariableAndDeclarationModules;
 	}
 }

test/cli/samples/watch/watch-config-early-update/_config.js

@@ -60,7 +60,7 @@ module.exports = {
 		        `
 					);
 					fs.writeFileSync(messageFile, 'loaded');
-				}, 300);
+				}, 500);
 			}
 		});
 	},
@@ -69,7 +69,7 @@ module.exports = {
 	},
 	abortOnStderr(data) {
 		if (reloadTriggered && data.includes('created _actual')) {
-			return new Promise(resolve => setTimeout(() => resolve(true), 300));
+			return new Promise(resolve => setTimeout(() => resolve(true), 500));
 		} else if (data.includes('Reloading updated config')) {
 			reloadTriggered = true;
 			return false;

test/form/samples/side-effect-default-reexport/_config.js

@@ -0,0 +1,7 @@
+module.exports = {
+	description:
+		'Observes side-effects in side-effect-free modules that contain a used default export that just reexports from another module',
+	options: {
+		treeshake: { moduleSideEffects: false }
+	}
+};

test/form/samples/side-effect-default-reexport/_expected.js

@@ -0,0 +1,32 @@
+var Menu = {
+	name: 'menu'
+};
+
+var Item = {
+	name: 'item'
+};
+
+Menu.Item1 = Item;
+
+Menu.Item2 = Item;
+
+var NamedExport = {
+	name: 'menu'
+};
+
+var Menu$1 = {
+	name: 'menu'
+};
+
+var Item$1 = {
+	name: 'item'
+};
+
+Menu$1.Item1 = Item$1;
+
+Menu$1.Item2 = Item$1;
+
+console.log('test-package-default-export', Menu.Item);
+console.log('test-package-named-export', NamedExport.Item);
+
+export default Menu$1;

test/form/samples/side-effect-default-reexport/default-export/index.js

@@ -0,0 +1,6 @@
+import Menu from './index2';
+import Item from './item';
+
+Menu.Item2 = Item;
+
+export default Menu;

test/form/samples/side-effect-default-reexport/default-export/index2.js

@@ -0,0 +1,6 @@
+import Menu from './menu';
+import Item from './item';
+
+Menu.Item1 = Item;
+
+export default Menu;

test/form/samples/side-effect-default-reexport/default-export/item.js

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

test/form/samples/side-effect-default-reexport/default-export/menu.js

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

test/form/samples/side-effect-default-reexport/default-export2/index.js

@@ -0,0 +1,6 @@
+import Menu from './index2';
+import Item from './item';
+
+Menu.Item2 = Item;
+
+export default Menu;

test/form/samples/side-effect-default-reexport/default-export2/index2.js

@@ -0,0 +1,6 @@
+import Menu from './menu';
+import Item from './item';
+
+Menu.Item1 = Item;
+
+export default Menu;

test/form/samples/side-effect-default-reexport/default-export2/item.js

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

test/form/samples/side-effect-default-reexport/default-export2/menu.js

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

test/form/samples/side-effect-default-reexport/main.js

@@ -0,0 +1,8 @@
+import DefaultExport from './default-export/index.js';
+console.log('test-package-default-export', DefaultExport.Item);
+
+import { Menu as NamedExport } from './named-export/index.js';
+console.log('test-package-named-export', NamedExport.Item);
+
+export { default } from './default-export2/index.js';
+

test/form/samples/side-effect-default-reexport/named-export/index.js

@@ -0,0 +1,6 @@
+import { Menu } from './index2';
+import Item from './item';
+
+Menu.Item2 = Item;
+
+export { Menu };

test/form/samples/side-effect-default-reexport/named-export/index2.js

@@ -0,0 +1,6 @@
+import Menu from './menu';
+import Item from './item';
+
+Menu.Item1 = Item;
+
+export { Menu };

test/form/samples/side-effect-default-reexport/named-export/item.js

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

test/form/samples/side-effect-default-reexport/named-export/menu.js

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