Fix and improve default export alias logic (#3521)

* Create failing test

* Improve default export short-cut handling

* Improve names

* Improve coverage

Author: lukastaegert

src/Chunk.ts

@@ -769,15 +769,13 @@ export default class Chunk {
 		for (const dep of this.dependencies) {
 			const imports: ImportSpecifier[] = [];
 			for (const variable of this.imports) {
-				const renderedVariable =
-					variable instanceof ExportDefaultVariable ? variable.getOriginalVariable() : variable;
 				if (
 					(variable.module instanceof Module
 						? variable.module.chunk === dep
 						: variable.module === dep) &&
-					!renderedImports.has(renderedVariable)
+					!renderedImports.has(variable)
 				) {
-					renderedImports.add(renderedVariable);
+					renderedImports.add(variable);
 					imports.push({
 						imported:
 							variable.module instanceof ExternalModule
@@ -1007,10 +1005,12 @@ export default class Chunk {
 
 	private setUpChunkImportsAndExportsForModule(module: Module) {
 		for (let variable of module.imports) {
+			if (variable instanceof SyntheticNamedExportVariable) {
+				variable = variable.getBaseVariable();
+			} else if (variable instanceof ExportDefaultVariable) {
+				variable = variable.getOriginalVariable();
+			}
 			if ((variable.module as Module).chunk !== this) {
-				if (variable instanceof SyntheticNamedExportVariable) {
-					variable = variable.getOriginalVariable();
-				}
 				this.imports.add(variable);
 				if (
 					!(variable instanceof NamespaceVariable && this.graph.preserveModules) &&
@@ -1029,7 +1029,7 @@ export default class Chunk {
 				this.exports.add(exportedVariable);
 				const isSynthetic = exportedVariable instanceof SyntheticNamedExportVariable;
 				const importedVariable = isSynthetic
-					? (exportedVariable as SyntheticNamedExportVariable).getOriginalVariable()
+					? (exportedVariable as SyntheticNamedExportVariable).getBaseVariable()
 					: exportedVariable;
 				const exportingModule = importedVariable.module;
 				if (

src/Module.ts

@@ -232,7 +232,7 @@ export default class Module {
 	private astContext!: AstContext;
 	private context: string;
 	private customTransformCache!: boolean;
-	private defaultExport: ExportDefaultVariable | null | undefined = null;
+	private defaultExport: Variable | null | undefined = null;
 	private esTreeAst!: acorn.Node;
 	private exportAllModules: (Module | ExternalModule)[] = [];
 	private exportNamesByVariable: Map<Variable, string[]> | null = null;
@@ -324,7 +324,7 @@ export default class Module {
 	getDefaultExport() {
 		if (this.defaultExport === null) {
 			this.defaultExport = undefined;
-			this.defaultExport = this.getVariableForExportName('default') as ExportDefaultVariable;
+			this.defaultExport = this.getVariableForExportName('default');
 		}
 		if (!this.defaultExport) {
 			return error({
@@ -339,21 +339,23 @@ export default class Module {
 	getDependenciesToBeIncluded(): Set<Module | ExternalModule> {
 		if (this.relevantDependencies) return this.relevantDependencies;
 		const relevantDependencies = new Set<Module | ExternalModule>();
-		for (const variable of this.imports) {
-			relevantDependencies.add(
-				variable instanceof SyntheticNamedExportVariable
-					? variable.getOriginalVariable().module!
-					: variable.module!
-			);
+		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!);
 		}
 		if (this.isEntryPoint || this.dynamicallyImportedBy.length > 0 || this.graph.preserveModules) {
 			for (const exportName of [...this.getReexports(), ...this.getExports()]) {
-				const variable = this.getVariableForExportName(exportName);
-				relevantDependencies.add(
-					variable instanceof SyntheticNamedExportVariable
-						? variable.getOriginalVariable().module!
-						: variable.module!
-				);
+				let variable = this.getVariableForExportName(exportName);
+				if (variable instanceof SyntheticNamedExportVariable) {
+					variable = variable.getBaseVariable();
+				} else if (variable instanceof ExportDefaultVariable) {
+					variable = variable.getOriginalVariable();
+				}
+				relevantDependencies.add(variable.module!);
 			}
 		}
 		if (this.graph.treeshakingOptions) {
@@ -402,7 +404,10 @@ export default class Module {
 		}
 		const exportNamesByVariable: Map<Variable, string[]> = new Map();
 		for (const exportName of this.getAllExportNames()) {
-			const tracedVariable = this.getVariableForExportName(exportName);
+			let tracedVariable = this.getVariableForExportName(exportName);
+			if (tracedVariable instanceof ExportDefaultVariable) {
+				tracedVariable = tracedVariable.getOriginalVariable();
+			}
 			if (
 				!tracedVariable ||
 				!(tracedVariable.included || tracedVariable instanceof ExternalVariable)

src/ast/nodes/ExportDefaultDeclaration.ts

@@ -17,13 +17,13 @@ import { ExpressionNode, IncludeChildren, NodeBase } from './shared/Node';
 const WHITESPACE = /\s/;
 
 // The header ends at the first non-white-space after "default"
-function getDeclarationStart(code: string, start = 0) {
+function getDeclarationStart(code: string, start: number) {
 	start = findFirstOccurrenceOutsideComment(code, 'default', start) + 7;
 	while (WHITESPACE.test(code[start])) start++;
 	return start;
 }
 
-function getIdInsertPosition(code: string, declarationKeyword: string, start = 0) {
+function getIdInsertPosition(code: string, declarationKeyword: string, start: number) {
 	const declarationEnd =
 		findFirstOccurrenceOutsideComment(code, declarationKeyword, start) + declarationKeyword.length;
 	code = code.slice(declarationEnd, findFirstOccurrenceOutsideComment(code, '{', declarationEnd));
@@ -84,15 +84,7 @@ export default class ExportDefaultDeclaration extends NodeBase {
 			);
 		} else if (this.variable.getOriginalVariable() !== this.variable) {
 			// Remove altogether to prevent re-declaring the same variable
-			if (options.format === 'system' && this.variable.exportName) {
-				code.overwrite(
-					start,
-					end,
-					`exports('${this.variable.exportName}', ${this.variable.getName()});`
-				);
-			} else {
-				treeshakeNode(this, code, start, end);
-			}
+			treeshakeNode(this, code, start, end);
 			return;
 		} else if (this.variable.included) {
 			this.renderVariableDeclaration(code, declarationStart, options);

src/ast/variables/ExportDefaultVariable.ts

@@ -73,22 +73,4 @@ export default class ExportDefaultVariable extends LocalVariable {
 		}
 		return this.originalVariable;
 	}
-
-	setRenderNames(baseName: string | null, name: string | null) {
-		const original = this.getOriginalVariable();
-		if (original === this) {
-			super.setRenderNames(baseName, name);
-		} else {
-			original.setRenderNames(baseName, name);
-		}
-	}
-
-	setSafeName(name: string | null) {
-		const original = this.getOriginalVariable();
-		if (original === this) {
-			super.setSafeName(name);
-		} else {
-			original.setSafeName(name);
-		}
-	}
 }

src/ast/variables/SyntheticNamedExportVariable.ts

@@ -1,29 +1,30 @@
 import Module, { AstContext } from '../../Module';
-import ExportDefaultVariable from './ExportDefaultVariable';
 import Variable from './Variable';
 
 export default class SyntheticNamedExportVariable extends Variable {
 	context: AstContext;
-	defaultVariable: ExportDefaultVariable;
+	defaultVariable: Variable;
 	module: Module;
 
-	constructor(context: AstContext, name: string, defaultVariable: ExportDefaultVariable) {
+	constructor(context: AstContext, name: string, defaultVariable: Variable) {
 		super(name);
 		this.context = context;
 		this.module = context.module;
 		this.defaultVariable = defaultVariable;
 	}
 
+	getBaseVariable(): Variable {
+		return this.defaultVariable instanceof SyntheticNamedExportVariable
+			? this.defaultVariable.getBaseVariable()
+			: this.defaultVariable;
+	}
+
 	getName(): string {
 		const name = this.name;
 		const renderBaseName = this.defaultVariable.getName();
 		return `${renderBaseName}${getPropertyAccess(name)}`;
 	}
 
-	getOriginalVariable(): Variable {
-		return this.defaultVariable.getOriginalVariable();
-	}
-
 	include() {
 		if (!this.included) {
 			this.included = true;

test/chunking-form/samples/chunk-deshadowing-reassignment/_expected/system/generated-dep1.js

@@ -3,8 +3,8 @@ System.register([], function (exports) {
 	return {
 		execute: function () {
 
-			var x = 42;
-			exports('x', x);console.log('dep1');
+			var x = exports('x', 42);
+			console.log('dep1');
 
 		}
 	};

test/chunking-form/samples/chunk-deshadowing-reassignment/_expected/system/generated-dep2.js

@@ -3,8 +3,8 @@ System.register([], function (exports) {
 	return {
 		execute: function () {
 
-			var x = 43;
-			exports('x', x);console.log('dep2');
+			var x = exports('x', 43);
+			console.log('dep2');
 
 		}
 	};

test/chunking-form/samples/chunk-namespace-boundary/_expected/amd/generated-shared.js

@@ -6,6 +6,6 @@ define(['exports'], function (exports) { 'use strict';
         var shared = commonjsGlobal.data;
 
         exports.commonjsGlobal = commonjsGlobal;
-        exports.d = shared;
+        exports.shared = shared;
 
 });

test/chunking-form/samples/chunk-namespace-boundary/_expected/amd/main1.js

@@ -3,7 +3,7 @@ define(['./generated-shared'], function (shared) { 'use strict';
 	shared.commonjsGlobal.fn = d => d + 1;
 	var cjs = shared.commonjsGlobal.fn;
 
-	var main1 = shared.d.map(cjs);
+	var main1 = shared.shared.map(cjs);
 
 	return main1;
 

test/chunking-form/samples/chunk-namespace-boundary/_expected/amd/main2.js

@@ -1,6 +1,6 @@
 define(['./generated-shared'], function (shared) { 'use strict';
 
-	var main2 = shared.d.map(d => d + 2);
+	var main2 = shared.shared.map(d => d + 2);
 
 	return main2;