Cache transitive reexport detection (#2864)

* Make test actually test the issue that was fixed, add basic caching for reexports

* Refine caching and add a side-effect to the test

src/Chunk.ts

@@ -337,8 +337,7 @@ export default class Chunk {
 		const dependencies: Set<Chunk | ExternalModule> = new Set();
 		const dynamicDependencies: Set<Chunk | ExternalModule> = new Set();
 		for (const module of this.orderedModules) {
-			this.addChunksFromDependencies(module.dependencies, dependencies);
-			this.addChunksFromDependencies(module.getReexportModules(), dependencies);
+			this.addChunksFromDependencies(module.getTransitiveDependencies(), dependencies);
 			this.addChunksFromDependencies(module.dynamicDependencies, dynamicDependencies);
 			this.setUpModuleImports(module);
 		}

src/Module.ts

@@ -212,6 +212,7 @@ export default class Module {
 	private magicString: MagicString;
 	private namespaceVariable: NamespaceVariable = undefined;
 	private transformDependencies: string[];
+	private transitiveReexports: string[];
 
 	constructor(graph: Graph, id: string, moduleSideEffects: boolean, isEntry: boolean) {
 		this.id = id;
@@ -311,37 +312,27 @@ export default class Module {
 		);
 	}
 
-	getReexportModules() {
-		return this.getReexports().map(exportName => this.getVariableForExportName(exportName).module);
-	}
-
-	getReexports(walkedModuleIds = new Set<string>()) {
-		// avoid infinite recursion when using circular `export * from X`
-		if (walkedModuleIds.has(this.id)) {
-			return [];
+	getReexports(): string[] {
+		if (this.transitiveReexports) {
+			return this.transitiveReexports;
 		}
-		walkedModuleIds.add(this.id);
-
-		const reexports = Object.create(null);
+		// to avoid infinite recursion when using circular `export * from X`
+		this.transitiveReexports = [];
 
+		const reexports = new Set<string>();
 		for (const name in this.reexports) {
-			reexports[name] = true;
+			reexports.add(name);
 		}
-
-		this.exportAllModules.forEach(module => {
-			if (module.isExternal) {
-				reexports[`*${module.id}`] = true;
-				return;
-			}
-
-			for (const name of (<Module>module)
-				.getExports()
-				.concat((<Module>module).getReexports(walkedModuleIds))) {
-				if (name !== 'default') reexports[name] = true;
+		for (const module of this.exportAllModules) {
+			if (module instanceof ExternalModule) {
+				reexports.add(`*${module.id}`);
+			} else {
+				for (const name of module.getExports().concat(module.getReexports())) {
+					if (name !== 'default') reexports.add(name);
+				}
 			}
-		});
-
-		return Object.keys(reexports);
+		}
+		return (this.transitiveReexports = Array.from(reexports));
 	}
 
 	getRenderedExports() {
@@ -355,6 +346,12 @@ export default class Module {
 		return { renderedExports, removedExports };
 	}
 
+	getTransitiveDependencies() {
+		return this.dependencies.concat(
+			this.getReexports().map(exportName => this.getVariableForExportName(exportName).module)
+		);
+	}
+
 	getVariableForExportName(name: string, isExportAllSearch?: boolean): Variable | null {
 		if (name[0] === '*') {
 			if (name.length === 1) {

test/chunking-form/samples/preserve-modules-nested-export/_config.js

@@ -1,7 +1,7 @@
 module.exports = {
 	description: 'confirm exports are preserved when exporting a module',
 	options: {
-		input: ['main1.js', 'main2.js'],
+		input: 'main.js',
 		preserveModules: true
 	}
 };

test/chunking-form/samples/preserve-modules-nested-export/_expected/amd/inner/more_inner/something.js

@@ -0,0 +1,7 @@
+define(['exports'], function (exports) { 'use strict';
+
+	const Something = 'Hello World';
+
+	exports.Something = Something;
+
+});

test/chunking-form/samples/preserve-modules-nested-export/_expected/amd/inner/some_effect.js

@@ -0,0 +1,5 @@
+define(function () { 'use strict';
+
+	console.log('side-effect');
+
+});

test/chunking-form/samples/preserve-modules-nested-export/_expected/amd/main.js

@@ -0,0 +1,9 @@
+define(['exports', './inner/more_inner/something', './inner/some_effect'], function (exports, __chunk_1, __chunk_3) { 'use strict';
+
+
+
+	exports.Something = __chunk_1.Something;
+
+	Object.defineProperty(exports, '__esModule', { value: true });
+
+});

test/chunking-form/samples/preserve-modules-nested-export/_expected/cjs/inner/more_inner/something.js

@@ -0,0 +1,5 @@
+'use strict';
+
+const Something = 'Hello World';
+
+exports.Something = Something;

test/chunking-form/samples/preserve-modules-nested-export/_expected/cjs/inner/some_effect.js

@@ -0,0 +1,3 @@
+'use strict';
+
+console.log('side-effect');

test/chunking-form/samples/preserve-modules-nested-export/_expected/cjs/main.js

@@ -0,0 +1,10 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', { value: true });
+
+var __chunk_1 = require('./inner/more_inner/something.js');
+require('./inner/some_effect.js');
+
+
+
+exports.Something = __chunk_1.Something;

test/chunking-form/samples/preserve-modules-nested-export/_expected/es/inner/more_inner/something.js

@@ -0,0 +1,3 @@
+const Something = 'Hello World';
+
+export { Something };

test/chunking-form/samples/preserve-modules-nested-export/_expected/es/inner/some_effect.js

@@ -0,0 +1 @@
+console.log('side-effect');

test/chunking-form/samples/preserve-modules-nested-export/_expected/es/main.js

@@ -0,0 +1,2 @@
+export { Something } from './inner/more_inner/something.js';
+import './inner/some_effect.js';

test/chunking-form/samples/preserve-modules-nested-export/_expected/system/inner/more_inner/something.js

@@ -0,0 +1,10 @@
+System.register([], function (exports, module) {
+	'use strict';
+	return {
+		execute: function () {
+
+			const Something = exports('Something', 'Hello World');
+
+		}
+	};
+});

test/chunking-form/samples/preserve-modules-nested-export/_expected/system/dep.js → test/chunking-form/samples/preserve-modules-nested-export/_expected/system/inner/some_effect.js

@@ -3,7 +3,7 @@ System.register([], function (exports, module) {
 	return {
 		execute: function () {
 
-			const foo = exports('foo', 1);
+			console.log('side-effect');
 
 		}
 	};

test/chunking-form/samples/preserve-modules-nested-export/_expected/system/main.js

@@ -0,0 +1,13 @@
+System.register(['./inner/more_inner/something.js', './inner/some_effect.js'], function (exports, module) {
+	'use strict';
+	return {
+		setters: [function (module) {
+			exports('Something', module.Something);
+		}, function () {}],
+		execute: function () {
+
+
+
+		}
+	};
+});

test/chunking-form/samples/preserve-modules-nested-export/inner/index.js

@@ -0,0 +1,2 @@
+export * from './more_inner/index';
+import './some_effect';

test/chunking-form/samples/preserve-modules-nested-export/inner/more_inner/index.js

@@ -0,0 +1 @@
+export * from './something';

test/chunking-form/samples/preserve-modules-nested-export/inner/more_inner/something.js

@@ -0,0 +1 @@
+export const Something = 'Hello World';

test/chunking-form/samples/preserve-modules-nested-export/inner/some_effect.js

@@ -0,0 +1 @@
+console.log('side-effect');

test/chunking-form/samples/preserve-modules-nested-export/main.js

@@ -0,0 +1 @@
+export * from './inner/index';