Prevent unneeded exports when entry facades are created and ensure al…

…l exported variables in facades are imported (#3590) * Prevent unneeded exports when entry facades are created and ensure all exported variables in facades are imported * Add test for namespaces with default reexport
rollup · May 24, 2020 · c126c94 · c126c94
1 parent 36a4527
commit c126c94
Show file tree

Hide file tree

Showing 65 changed files with 577 additions and 62 deletions.
diff --git a/src/Chunk.ts b/src/Chunk.ts
@@ -136,6 +136,7 @@ export default class Chunk {
 		) {
 			chunk.dependencies.add(facadedModule.chunk!);
 		}
+		chunk.ensureReexportsAreAvailableForModule(facadedModule);
 		chunk.facadeModule = facadedModule;
 		chunk.strictFacade = true;
 		return chunk;
@@ -214,7 +215,7 @@ export default class Chunk {
 		}
 	}
 
-	canModuleBeFacade(module: Module, exposedNamespaces: NamespaceVariable[]): boolean {
+	canModuleBeFacade(module: Module, exposedVariables: Set<Variable>): boolean {
 		const moduleExportNamesByVariable = module.getExportNamesByVariable();
 		for (const exposedVariable of this.exports) {
 			if (!moduleExportNamesByVariable.has(exposedVariable)) {
@@ -236,7 +237,7 @@ export default class Chunk {
 				return false;
 			}
 		}
-		for (const exposedVariable of exposedNamespaces) {
+		for (const exposedVariable of exposedVariables) {
 			if (
 				!(moduleExportNamesByVariable.has(exposedVariable) || exposedVariable.module === module)
 			) {
@@ -279,7 +280,14 @@ export default class Chunk {
 			module.includedDynamicImporters.some(importingModule => importingModule.chunk !== this)
 		);
 		this.isDynamicEntry = dynamicEntryModules.length > 0;
-		const exposedNamespaces = dynamicEntryModules.map(module => module.namespace);
+		const exposedVariables = new Set<Variable>(dynamicEntryModules.map(module => module.namespace));
+		for (const module of this.entryModules) {
+			if (module.preserveSignature) {
+				for (const exportedVariable of module.getExportNamesByVariable().keys()) {
+					exposedVariables.add(exportedVariable);
+				}
+			}
+		}
 		for (const module of this.entryModules) {
 			const requiredFacades: FacadeName[] = [...module.userChunkNames].map(name => ({
 				name
@@ -295,11 +303,14 @@ export default class Chunk {
 				!this.facadeModule &&
 				(this.graph.preserveModules ||
 					module.preserveSignature !== 'strict' ||
-					this.canModuleBeFacade(module, exposedNamespaces))
+					this.canModuleBeFacade(module, exposedVariables))
 			) {
 				this.facadeModule = module;
 				module.facadeChunk = this;
-				this.strictFacade = module.preserveSignature === 'strict';
+				if (module.preserveSignature) {
+					this.strictFacade = module.preserveSignature === 'strict';
+					this.ensureReexportsAreAvailableForModule(module);
+				}
 				this.assignFacadeName(requiredFacades.shift()!, module);
 			}
 
@@ -308,15 +319,15 @@ export default class Chunk {
 			}
 		}
 		for (const module of dynamicEntryModules) {
-			if (!this.facadeModule && this.canModuleBeFacade(module, exposedNamespaces)) {
+			if (!this.facadeModule && this.canModuleBeFacade(module, exposedVariables)) {
 				this.facadeModule = module;
 				module.facadeChunk = this;
 				this.strictFacade = true;
 				this.assignFacadeName({}, module);
 			} else if (
 				this.facadeModule === module &&
 				!this.strictFacade &&
-				this.canModuleBeFacade(module, exposedNamespaces)
+				this.canModuleBeFacade(module, exposedVariables)
 			) {
 				this.strictFacade = true;
 			} else if (!(module.facadeChunk && module.facadeChunk.strictFacade)) {
@@ -749,6 +760,28 @@ export default class Chunk {
 		return hash.digest('hex').substr(0, 8);
 	}
 
+	private ensureReexportsAreAvailableForModule(module: Module) {
+		const map = module.getExportNamesByVariable();
+		for (const exportedVariable of map.keys()) {
+			const isSynthetic = exportedVariable instanceof SyntheticNamedExportVariable;
+			const importedVariable = isSynthetic
+				? (exportedVariable as SyntheticNamedExportVariable).getBaseVariable()
+				: exportedVariable;
+			const exportingModule = importedVariable.module;
+			if (
+				exportingModule &&
+				exportingModule.chunk &&
+				exportingModule.chunk !== this &&
+				!(importedVariable instanceof NamespaceVariable && this.graph.preserveModules)
+			) {
+				exportingModule.chunk.exports.add(importedVariable);
+				if (isSynthetic) {
+					this.imports.add(importedVariable);
+				}
+			}
+		}
+	}
+
 	private finaliseDynamicImports(options: OutputOptions) {
 		const stripKnownJsExtensions = options.format === 'amd';
 		for (const [module, code] of this.renderedModuleSources) {
@@ -1067,11 +1100,8 @@ export default class Chunk {
 			if (variable instanceof ExportDefaultVariable) {
 				variable = variable.getOriginalVariable();
 			}
-			while (variable instanceof SyntheticNamedExportVariable) {
+			if (variable instanceof SyntheticNamedExportVariable) {
 				variable = variable.getBaseVariable();
-				if (variable instanceof ExportDefaultVariable) {
-					variable = variable.getOriginalVariable();
-				}
 			}
 			if (variable.module && (variable.module as Module).chunk !== this) {
 				this.imports.add(variable);
@@ -1085,44 +1115,21 @@ export default class Chunk {
 		}
 		if (
 			(module.isEntryPoint && module.preserveSignature !== false) ||
-			module.includedDynamicImporters.some(importer => importer.chunk !== this)
+			module.includedDynamicImporters.some(importer => importer.chunk !== this) ||
+			module.namespace.included
 		) {
-			const map = module.getExportNamesByVariable();
-			for (const exportedVariable of map.keys()) {
-				if (module.isEntryPoint && module.preserveSignature !== false) {
-					this.exports.add(exportedVariable);
-				}
-				const isSynthetic = exportedVariable instanceof SyntheticNamedExportVariable;
-				const importedVariable = isSynthetic
-					? (exportedVariable as SyntheticNamedExportVariable).getBaseVariable()
-					: exportedVariable;
-				const exportingModule = importedVariable.module;
-				if (
-					exportingModule &&
-					exportingModule.chunk &&
-					exportingModule.chunk !== this &&
-					!(importedVariable instanceof NamespaceVariable && this.graph.preserveModules)
-				) {
-					exportingModule.chunk.exports.add(importedVariable);
-					if (isSynthetic) {
-						this.imports.add(importedVariable);
-					}
-				}
-			}
-		}
-		if (module.namespace.included) {
-			for (const reexportName of Object.keys(module.reexportDescriptions)) {
-				const reexport = module.reexportDescriptions[reexportName];
-				const variable = reexport.module.getVariableForExportName(reexport.localName);
-				if ((variable.module as Module).chunk && (variable.module as Module).chunk !== this) {
-					this.imports.add(variable);
-					(variable.module as Module).chunk!.exports.add(variable);
-				}
-			}
+			this.ensureReexportsAreAvailableForModule(module);
 		}
 		for (const { node, resolution } of module.dynamicImports) {
-			if (node.included && resolution instanceof Module && resolution.chunk === this)
+			if (
+				node.included &&
+				resolution instanceof Module &&
+				resolution.chunk === this &&
+				!resolution.namespace.included
+			) {
 				resolution.namespace.include();
+				this.ensureReexportsAreAvailableForModule(resolution);
+			}
 		}
 	}
 }
diff --git a/src/ast/variables/SyntheticNamedExportVariable.ts b/src/ast/variables/SyntheticNamedExportVariable.ts
@@ -1,4 +1,5 @@
 import Module, { AstContext } from '../../Module';
+import ExportDefaultVariable from './ExportDefaultVariable';
 import Variable from './Variable';
 
 export default class SyntheticNamedExportVariable extends Variable {
@@ -14,9 +15,14 @@ export default class SyntheticNamedExportVariable extends Variable {
 	}
 
 	getBaseVariable(): Variable {
-		return this.defaultVariable instanceof SyntheticNamedExportVariable
-			? this.defaultVariable.getBaseVariable()
-			: this.defaultVariable;
+		let baseVariable = this.defaultVariable;
+		if (baseVariable instanceof ExportDefaultVariable) {
+			baseVariable = baseVariable.getOriginalVariable();
+		}
+		if (baseVariable instanceof SyntheticNamedExportVariable) {
+			baseVariable = baseVariable.getBaseVariable();
+		}
+		return baseVariable;
 	}
 
 	getName(): string {

diff --git a/test/chunking-form/samples/circular-entry-points2/_config.js b/test/chunking-form/samples/circular-entry-points2/_config.js
@@ -0,0 +1,7 @@
+module.exports = {
+	description: 'does not create a facade for one circular entry point if possible',
+	expectedWarnings: ['CIRCULAR_DEPENDENCY'],
+	options: {
+		input: ['main1.js', 'main2.js']
+	}
+};
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/amd/main1.js b/test/chunking-form/samples/circular-entry-points2/_expected/amd/main1.js
@@ -0,0 +1,9 @@
+define(['exports', './main2'], function (exports, main2) { 'use strict';
+
+
+
+	exports.p = main2.p2;
+
+	Object.defineProperty(exports, '__esModule', { value: true });
+
+});
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/amd/main2.js b/test/chunking-form/samples/circular-entry-points2/_expected/amd/main2.js
@@ -0,0 +1,28 @@
+define(['exports'], function (exports) { 'use strict';
+
+  class C {
+    fn (num) {
+      console.log(num - p$1);
+    }
+  }
+
+  var p = 43;
+
+  new C().fn(p);
+
+  class C$1 {
+    fn (num) {
+      console.log(num - p);
+    }
+  }
+
+  var p$1 = 42;
+
+  new C$1().fn(p$1);
+
+  exports.p = p;
+  exports.p2 = p$1;
+
+  Object.defineProperty(exports, '__esModule', { value: true });
+
+});
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/cjs/main1.js b/test/chunking-form/samples/circular-entry-points2/_expected/cjs/main1.js
@@ -0,0 +1,9 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', { value: true });
+
+var main2 = require('./main2.js');
+
+
+
+exports.p = main2.p2;
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/cjs/main2.js b/test/chunking-form/samples/circular-entry-points2/_expected/cjs/main2.js
@@ -0,0 +1,26 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', { value: true });
+
+class C {
+  fn (num) {
+    console.log(num - p$1);
+  }
+}
+
+var p = 43;
+
+new C().fn(p);
+
+class C$1 {
+  fn (num) {
+    console.log(num - p);
+  }
+}
+
+var p$1 = 42;
+
+new C$1().fn(p$1);
+
+exports.p = p;
+exports.p2 = p$1;
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/es/main1.js b/test/chunking-form/samples/circular-entry-points2/_expected/es/main1.js
@@ -0,0 +1 @@
+export { p2 as p } from './main2.js';
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/es/main2.js b/test/chunking-form/samples/circular-entry-points2/_expected/es/main2.js
@@ -0,0 +1,21 @@
+class C {
+  fn (num) {
+    console.log(num - p$1);
+  }
+}
+
+var p = 43;
+
+new C().fn(p);
+
+class C$1 {
+  fn (num) {
+    console.log(num - p);
+  }
+}
+
+var p$1 = 42;
+
+new C$1().fn(p$1);
+
+export { p, p$1 as p2 };
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/system/main1.js b/test/chunking-form/samples/circular-entry-points2/_expected/system/main1.js
@@ -0,0 +1,13 @@
+System.register(['./main2.js'], function (exports) {
+	'use strict';
+	return {
+		setters: [function (module) {
+			exports('p', module.p2);
+		}],
+		execute: function () {
+
+
+
+		}
+	};
+});
diff --git a/test/chunking-form/samples/circular-entry-points2/_expected/system/main2.js b/test/chunking-form/samples/circular-entry-points2/_expected/system/main2.js
@@ -0,0 +1,28 @@
+System.register([], function (exports) {
+  'use strict';
+  return {
+    execute: function () {
+
+      class C {
+        fn (num) {
+          console.log(num - p$1);
+        }
+      }
+
+      var p = exports('p', 43);
+
+      new C().fn(p);
+
+      class C$1 {
+        fn (num) {
+          console.log(num - p);
+        }
+      }
+
+      var p$1 = exports('p2', 42);
+
+      new C$1().fn(p$1);
+
+    }
+  };
+});
diff --git a/test/chunking-form/samples/circular-entry-points2/dep1.js b/test/chunking-form/samples/circular-entry-points2/dep1.js
@@ -0,0 +1,7 @@
+import { p } from './main2.js';
+
+export class C {
+  fn (num) {
+    console.log(num - p);
+  }
+}
diff --git a/test/chunking-form/samples/circular-entry-points2/dep2.js b/test/chunking-form/samples/circular-entry-points2/dep2.js
@@ -0,0 +1,7 @@
+import { p } from './main1.js';
+
+export class C {
+  fn (num) {
+    console.log(num - p);
+  }
+}
diff --git a/test/chunking-form/samples/circular-entry-points2/main1.js b/test/chunking-form/samples/circular-entry-points2/main1.js
@@ -0,0 +1,4 @@
+import { C } from './dep1.js';
+export var p = 42;
+
+new C().fn(p);
diff --git a/test/chunking-form/samples/circular-entry-points2/main2.js b/test/chunking-form/samples/circular-entry-points2/main2.js
@@ -0,0 +1,5 @@
+import { C } from './dep2.js';
+export var p = 43;
+export {p as p2} from './main1'
+
+new C().fn(p);
diff --git a/test/chunking-form/samples/circular-entry-points3/_config.js b/test/chunking-form/samples/circular-entry-points3/_config.js
@@ -0,0 +1,8 @@
+module.exports = {
+	description:
+		'creates facades for all circular entry points if they become tainted by another entry',
+	expectedWarnings: ['CIRCULAR_DEPENDENCY'],
+	options: {
+		input: ['main1.js', 'main2.js', 'main3.js']
+	}
+};