refactor: use map for namespace reexports by name (#4411)

dnalborczyk · lukastaegert · web-flow · commit 3106ec99cc1e · 2022-02-22T06:04:41.000Z
* refactor: use map for namespace reexports by name * remove explicit type assignment * refactor: remove variable, use context property directly * refactor: remove unnecessary array assignment * Add test for recursive reexports * Revert "refactor: remove unnecessary array assignment" This reverts commit f7434c3. Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com> Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
diff --git a/src/Module.ts b/src/Module.ts
@@ -232,13 +232,13 @@ export default class Module {
 	private readonly exportAllModules: (Module | ExternalModule)[] = [];
 	private readonly exportAllSources = new Set<string>();
 	private exportNamesByVariable: Map<Variable, string[]> | null = null;
-	private readonly exportShimVariable: ExportShimVariable = new ExportShimVariable(this);
+	private readonly exportShimVariable = new ExportShimVariable(this);
 	private readonly exports = new Map<string, ExportDescription>();
 	private declare magicString: MagicString;
-	private namespaceReexportsByName: Record<
+	private readonly namespaceReexportsByName = new Map<
 		string,
 		[variable: Variable | null, indirectExternal?: boolean]
-	> = Object.create(null);
+	>();
 	private readonly reexportDescriptions = new Map<string, ReexportDescription>();
 	private relevantDependencies: Set<Module | ExternalModule> | null = null;
 	private readonly syntheticExports = new Map<string, SyntheticNamedExportVariable>();
@@ -380,7 +380,8 @@ export default class Module {
 
 	getDependenciesToBeIncluded(): Set<Module | ExternalModule> {
 		if (this.relevantDependencies) return this.relevantDependencies;
-		const relevantDependencies = new Set<Module | ExternalModule>();
+
+		this.relevantDependencies = new Set<Module | ExternalModule>();
 		const necessaryDependencies = new Set<Module | ExternalModule>();
 		const alwaysCheckedDependencies = new Set<Module>();
 		const dependencyVariables = new Set(this.imports);
@@ -414,19 +415,19 @@ export default class Module {
 		}
 		if (!this.options.treeshake || this.info.moduleSideEffects === 'no-treeshake') {
 			for (const dependency of this.dependencies) {
-				relevantDependencies.add(dependency);
+				this.relevantDependencies.add(dependency);
 			}
 		} else {
 			this.addRelevantSideEffectDependencies(
-				relevantDependencies,
+				this.relevantDependencies,
 				necessaryDependencies,
 				alwaysCheckedDependencies
 			);
 		}
 		for (const dependency of necessaryDependencies) {
-			relevantDependencies.add(dependency);
+			this.relevantDependencies.add(dependency);
 		}
-		return (this.relevantDependencies = relevantDependencies);
+		return this.relevantDependencies;
 	}
 
 	getExportNamesByVariable(): Map<Variable, string[]> {
@@ -579,14 +580,13 @@ export default class Module {
 
 		if (name !== 'default') {
 			const foundNamespaceReexport =
-				name in this.namespaceReexportsByName
-					? this.namespaceReexportsByName[name]
-					: this.getVariableFromNamespaceReexports(
-							name,
-							importerForSideEffects,
-							searchedNamesAndModules
-					  );
-			this.namespaceReexportsByName[name] = foundNamespaceReexport;
+				this.namespaceReexportsByName.get(name) ??
+				this.getVariableFromNamespaceReexports(
+					name,
+					importerForSideEffects,
+					searchedNamesAndModules
+				);
+			this.namespaceReexportsByName.set(name, foundNamespaceReexport);
 			if (foundNamespaceReexport[0]) {
 				return foundNamespaceReexport;
 			}
diff --git a/test/function/samples/recursive-reexports/_config.js b/test/function/samples/recursive-reexports/_config.js
@@ -0,0 +1,15 @@
+const assert = require('assert');
+module.exports = {
+	description: 'handles recursive namespace reexports',
+	exports(exports) {
+		assert.deepStrictEqual(exports, { main: 'main', other: 'other' });
+	},
+	warnings: [
+		{
+			code: 'CIRCULAR_DEPENDENCY',
+			cycle: ['main.js', 'other.js', 'main.js'],
+			importer: 'main.js',
+			message: 'Circular dependency: main.js -> other.js -> main.js'
+		}
+	]
+};
diff --git a/test/function/samples/recursive-reexports/main.js b/test/function/samples/recursive-reexports/main.js
@@ -0,0 +1,2 @@
+export * from './other.js';
+export const main = 'main';
diff --git a/test/function/samples/recursive-reexports/other.js b/test/function/samples/recursive-reexports/other.js
@@ -0,0 +1,2 @@
+export * from './main.js';
+export const other = 'other';

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+export * from './other.js';`
	`2`	`+export const main = 'main';`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+export * from './main.js';`
	`2`	`+export const other = 'other';`