From 3106ec99cc1e516f081c9707352b2f159f518cf8 Mon Sep 17 00:00:00 2001
From: dnalborczyk <dnalborczyk@gmail.com>
Date: Tue, 22 Feb 2022 01:04:41 -0500
Subject: [PATCH] refactor: use map for namespace reexports by name (#4411)

* 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 f7434c3453969463d600ca56ed974fc930764888.

Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>
Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
---
 src/Module.ts                                 | 32 +++++++++----------
 .../samples/recursive-reexports/_config.js    | 15 +++++++++
 .../samples/recursive-reexports/main.js       |  2 ++
 .../samples/recursive-reexports/other.js      |  2 ++
 4 files changed, 35 insertions(+), 16 deletions(-)
 create mode 100644 test/function/samples/recursive-reexports/_config.js
 create mode 100644 test/function/samples/recursive-reexports/main.js
 create mode 100644 test/function/samples/recursive-reexports/other.js

diff --git a/src/Module.ts b/src/Module.ts
index 2743a56961e..0ce3ef4e39b 100644
--- 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
new file mode 100644
index 00000000000..1e1ce84b159
--- /dev/null
+++ 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
new file mode 100644
index 00000000000..aa89b491fd7
--- /dev/null
+++ 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
new file mode 100644
index 00000000000..3c111285103
--- /dev/null
+++ b/test/function/samples/recursive-reexports/other.js
@@ -0,0 +1,2 @@
+export * from './main.js';
+export const other = 'other';