Handle some cases of duplicate export bindings (#3575)

* Handle some cases of inlined exports and tests. * Consolidate test cases for discussion * fixup systen destructuring default assignment * fixup edge cases * wat * remove safeExportName, exportName -> exportNames * First solution: Reset exportNames. Potentially dangerous. * Remove `exportNames` property altogether and replace it by a map on Chunk to avoid interactions between Chunks that export the same variables, and interactions between outputs * Polish output cases * remove unused * ws refactor * further ws refactor * fixup assign space * Fix linting * Improve coverage * Improve coverage * further refinements * further refinements, converage * fixup imports * Improve coverage * refactor ws handling * inline variable * scanWs refactor * Make name somewhat clearer Co-authored-by: Joel Jeske <jjeske@netspend.com> Co-authored-by: Guy Bedford <guybedford@gmail.com> Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
rollup · Jun 7, 2020 · 733815c · 733815c
1 parent e2618c8
commit 733815c
Show file tree

Hide file tree

Showing 48 changed files with 476 additions and 168 deletions.
diff --git a/src/Chunk.ts b/src/Chunk.ts
@@ -151,8 +151,9 @@ export default class Chunk {
 	private dependencies = new Set<ExternalModule | Chunk>();
 	private dynamicDependencies = new Set<ExternalModule | Chunk>();
 	private dynamicEntryModules: Module[] = [];
+	private exportNamesByVariable: Map<Variable, string[]> | null = null;
 	private exports = new Set<Variable>();
-	private exportsByName: Record<string, Variable> = Object.create(null);
+	private exportsByName: Record<string, Variable> | null = null;
 	private fileName: string | null = null;
 	private implicitEntryModules: Module[] = [];
 	private implicitlyLoadedBefore = new Set<Chunk>();
@@ -258,23 +259,29 @@ export default class Chunk {
 	generateExports(options: NormalizedOutputOptions) {
 		this.sortedExportNames = null;
 		this.exportsByName = Object.create(null);
+		this.exportNamesByVariable = new Map();
 		const remainingExports = new Set(this.exports);
 		if (
 			this.facadeModule !== null &&
 			(this.facadeModule.preserveSignature !== false || this.strictFacade)
 		) {
 			const exportNamesByVariable = this.facadeModule.getExportNamesByVariable();
 			for (const [variable, exportNames] of exportNamesByVariable) {
+				this.exportNamesByVariable.set(variable, [...exportNames]);
 				for (const exportName of exportNames) {
-					this.exportsByName[exportName] = variable;
+					this.exportsByName![exportName] = variable;
 				}
 				remainingExports.delete(variable);
 			}
 		}
 		if (options.minifyInternalExports) {
-			assignExportsToMangledNames(remainingExports, this.exportsByName);
+			assignExportsToMangledNames(
+				remainingExports,
+				this.exportsByName!,
+				this.exportNamesByVariable
+			);
 		} else {
-			assignExportsToNames(remainingExports, this.exportsByName);
+			assignExportsToNames(remainingExports, this.exportsByName!, this.exportNamesByVariable);
 		}
 		if (this.inputOptions.preserveModules || (this.facadeModule && this.facadeModule.isEntryPoint))
 			this.exportMode = getExportMode(
@@ -425,7 +432,7 @@ export default class Chunk {
 
 	getExportNames(): string[] {
 		return (
-			this.sortedExportNames || (this.sortedExportNames = Object.keys(this.exportsByName).sort())
+			this.sortedExportNames || (this.sortedExportNames = Object.keys(this.exportsByName!).sort())
 		);
 	}
 
@@ -471,7 +478,7 @@ export default class Chunk {
 		hash.update(
 			this.getExportNames()
 				.map(exportName => {
-					const variable = this.exportsByName[exportName];
+					const variable = this.exportsByName![exportName];
 					return `${relativeId((variable.module as Module).id).replace(/\\/g, '/')}:${
 						variable.name
 					}:${exportName}`;
@@ -485,10 +492,7 @@ export default class Chunk {
 		if (this.inputOptions.preserveModules && variable instanceof NamespaceVariable) {
 			return '*';
 		}
-		for (const exportName of Object.keys(this.exportsByName)) {
-			if (this.exportsByName[exportName] === variable) return exportName;
-		}
-		throw new Error(`Internal Error: Could not find export name for variable ${variable.name}.`);
+		return this.exportNamesByVariable!.get(variable)![0];
 	}
 
 	link() {
@@ -514,6 +518,7 @@ export default class Chunk {
 		const renderOptions: RenderOptions = {
 			compact: options.compact,
 			dynamicImportFunction: options.dynamicImportFunction,
+			exportNamesByVariable: this.exportNamesByVariable!,
 			format: options.format,
 			freeze: options.freeze,
 			indent: this.indentString,
@@ -843,7 +848,7 @@ export default class Chunk {
 					renderedResolution,
 					resolution instanceof Module &&
 						!(resolution.facadeChunk && resolution.facadeChunk.strictFacade) &&
-						resolution.namespace.exportName!,
+						resolution.chunk!.exportNamesByVariable!.get(resolution.namespace)![0],
 					options
 				);
 			}
@@ -875,7 +880,7 @@ export default class Chunk {
 				exportChunk = this.modulesById.get(exportName.substr(1)) as ExternalModule;
 				importName = exportName = '*';
 			} else {
-				const variable = this.exportsByName[exportName];
+				const variable = this.exportsByName![exportName];
 				if (variable instanceof SyntheticNamedExportVariable) continue;
 				const module = variable.module;
 				if (!module || module.chunk === this) continue;
@@ -965,7 +970,7 @@ export default class Chunk {
 		for (const exportName of this.getExportNames()) {
 			if (exportName[0] === '*') continue;
 
-			const variable = this.exportsByName[exportName];
+			const variable = this.exportsByName![exportName];
 			if (!(variable instanceof SyntheticNamedExportVariable)) {
 				const module = variable.module;
 				if (module && module.chunk !== this) continue;
@@ -1063,13 +1068,11 @@ export default class Chunk {
 
 	private setIdentifierRenderResolutions(options: NormalizedOutputOptions) {
 		const syntheticExports = new Set<SyntheticNamedExportVariable>();
-
 		for (const exportName of this.getExportNames()) {
-			const exportVariable = this.exportsByName[exportName];
+			const exportVariable = this.exportsByName![exportName];
 			if (exportVariable instanceof ExportShimVariable) {
 				this.needsExportsShim = true;
 			}
-			exportVariable.exportName = exportName;
 			if (
 				options.format !== 'es' &&
 				options.format !== 'system' &&
@@ -1109,7 +1112,8 @@ export default class Chunk {
 			options.format as string,
 			options.interop,
 			this.inputOptions.preserveModules,
-			syntheticExports
+			syntheticExports,
+			this.exportNamesByVariable!
 		);
 	}
 

diff --git a/src/ast/nodes/ArrayPattern.ts b/src/ast/nodes/ArrayPattern.ts
@@ -10,10 +10,13 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
 	elements!: (PatternNode | null)[];
 	type!: NodeType.tArrayPattern;
 
-	addExportedVariables(variables: Variable[]): void {
+	addExportedVariables(
+		variables: Variable[],
+		exportNamesByVariable: Map<Variable, string[]>
+	): void {
 		for (const element of this.elements) {
 			if (element !== null) {
-				element.addExportedVariables(variables);
+				element.addExportedVariables(variables, exportNamesByVariable);
 			}
 		}
 	}

diff --git a/src/ast/nodes/AssignmentExpression.ts b/src/ast/nodes/AssignmentExpression.ts
@@ -1,6 +1,10 @@
 import MagicString from 'magic-string';
-import { findFirstOccurrenceOutsideComment, RenderOptions } from '../../utils/renderHelpers';
-import { getSystemExportStatement } from '../../utils/systemJsRendering';
+import {
+	findFirstOccurrenceOutsideComment,
+	findNonWhiteSpace,
+	RenderOptions
+} from '../../utils/renderHelpers';
+import { getSystemExportFunctionLeft } from '../../utils/systemJsRendering';
 import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import { EMPTY_PATH, ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
 import Variable from '../variables/Variable';
@@ -9,7 +13,7 @@ import { ExpressionNode, IncludeChildren, NodeBase } from './shared/Node';
 import { PatternNode } from './shared/Pattern';
 
 export default class AssignmentExpression extends NodeBase {
-	left!: PatternNode | ExpressionNode;
+	left!: PatternNode;
 	operator!:
 		| '='
 		| '+='
@@ -52,29 +56,34 @@ export default class AssignmentExpression extends NodeBase {
 		this.left.render(code, options);
 		this.right.render(code, options);
 		if (options.format === 'system') {
-			if (this.left.variable && this.left.variable.exportName) {
+			const exportNames =
+				this.left.variable && options.exportNamesByVariable.get(this.left.variable);
+			if (this.left.type === 'Identifier' && exportNames) {
+				const _ = options.compact ? '' : ' ';
 				const operatorPos = findFirstOccurrenceOutsideComment(
 					code.original,
 					this.operator,
 					this.left.end
 				);
 				const operation =
-					this.operator.length > 1
-						? ` ${this.left.variable.exportName} ${this.operator.slice(0, -1)}`
-						: '';
+					this.operator.length > 1 ? `${exportNames[0]}${_}${this.operator.slice(0, -1)}${_}` : '';
 				code.overwrite(
 					operatorPos,
-					operatorPos + this.operator.length,
-					`= exports('${this.left.variable.exportName}',${operation}`
+					findNonWhiteSpace(code.original, operatorPos + this.operator.length),
+					`=${_}${
+						exportNames.length === 1
+							? `exports('${exportNames[0]}',${_}`
+							: getSystemExportFunctionLeft([this.left.variable!], false, options)
+					}${operation}`
 				);
-				code.appendLeft(this.right.end, `)`);
-			} else if ('addExportedVariables' in this.left) {
+				code.appendLeft(this.right.end, ')');
+			} else {
 				const systemPatternExports: Variable[] = [];
-				this.left.addExportedVariables(systemPatternExports);
+				this.left.addExportedVariables(systemPatternExports, options.exportNamesByVariable);
 				if (systemPatternExports.length > 0) {
 					code.prependRight(
 						this.start,
-						`function (v) {${getSystemExportStatement(systemPatternExports)} return v;} (`
+						getSystemExportFunctionLeft(systemPatternExports, true, options)
 					);
 					code.appendLeft(this.end, ')');
 				}

diff --git a/src/ast/nodes/AssignmentPattern.ts b/src/ast/nodes/AssignmentPattern.ts
@@ -14,8 +14,11 @@ export default class AssignmentPattern extends NodeBase implements PatternNode {
 	right!: ExpressionNode;
 	type!: NodeType.tAssignmentPattern;
 
-	addExportedVariables(variables: Variable[]): void {
-		this.left.addExportedVariables(variables);
+	addExportedVariables(
+		variables: Variable[],
+		exportNamesByVariable: Map<Variable, string[]>
+	): void {
+		this.left.addExportedVariables(variables, exportNamesByVariable);
 	}
 
 	bind() {

diff --git a/src/ast/nodes/ClassDeclaration.ts b/src/ast/nodes/ClassDeclaration.ts
@@ -1,5 +1,6 @@
 import MagicString from 'magic-string';
 import { RenderOptions } from '../../utils/renderHelpers';
+import { getSystemExportStatement } from '../../utils/systemJsRendering';
 import ChildScope from '../scopes/ChildScope';
 import { IdentifierWithVariable } from './Identifier';
 import * as NodeType from './NodeType';
@@ -19,18 +20,22 @@ export default class ClassDeclaration extends ClassNode {
 
 	parseNode(esTreeNode: GenericEsTreeNode) {
 		if (esTreeNode.id !== null) {
-			this.id = new this.context.nodeConstructors.Identifier(esTreeNode.id, this, this.scope
-				.parent as ChildScope) as IdentifierWithVariable;
+			this.id = new this.context.nodeConstructors.Identifier(
+				esTreeNode.id,
+				this,
+				this.scope.parent as ChildScope
+			) as IdentifierWithVariable;
 		}
 		super.parseNode(esTreeNode);
 	}
 
 	render(code: MagicString, options: RenderOptions) {
-		if (options.format === 'system' && this.id && this.id.variable.exportName) {
-			code.appendLeft(
-				this.end,
-				` exports('${this.id.variable.exportName}', ${this.id.variable.getName()});`
-			);
+		if (
+			options.format === 'system' &&
+			this.id &&
+			options.exportNamesByVariable.has(this.id.variable)
+		) {
+			code.appendLeft(this.end, `${options.compact ? '' : ' '}${getSystemExportStatement([this.id.variable], options)};`);
 		}
 		super.render(code, options);
 	}

diff --git a/src/ast/nodes/ExportDefaultDeclaration.ts b/src/ast/nodes/ExportDefaultDeclaration.ts
@@ -1,9 +1,11 @@
 import MagicString from 'magic-string';
 import {
 	findFirstOccurrenceOutsideComment,
+	findNonWhiteSpace,
 	NodeRenderOptions,
 	RenderOptions
 } from '../../utils/renderHelpers';
+import { getSystemExportStatement } from '../../utils/systemJsRendering';
 import { treeshakeNode } from '../../utils/treeshakeNode';
 import { InclusionContext } from '../ExecutionContext';
 import ModuleScope from '../scopes/ModuleScope';
@@ -14,13 +16,9 @@ import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 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: number) {
-	start = findFirstOccurrenceOutsideComment(code, 'default', start) + 7;
-	while (WHITESPACE.test(code[start])) start++;
-	return start;
+	return findNonWhiteSpace(code, findFirstOccurrenceOutsideComment(code, 'default', start) + 7);
 }
 
 function getIdInsertPosition(
@@ -133,9 +131,9 @@ export default class ExportDefaultDeclaration extends NodeBase {
 		if (
 			options.format === 'system' &&
 			this.declaration instanceof ClassDeclaration &&
-			this.variable.exportName
+			options.exportNamesByVariable.has(this.variable)
 		) {
-			code.appendLeft(this.end, ` exports('${this.variable.exportName}', ${name});`);
+			code.appendLeft(this.end, ` ${getSystemExportStatement([this.variable], options)};`);
 		}
 	}
 
@@ -144,23 +142,29 @@ export default class ExportDefaultDeclaration extends NodeBase {
 		declarationStart: number,
 		options: RenderOptions
 	) {
-		const systemBinding =
-			options.format === 'system' && this.variable.exportName
-				? `exports('${this.variable.exportName}', `
-				: '';
-		code.overwrite(
-			this.start,
-			declarationStart,
-			`${options.varOrConst} ${this.variable.getName()} = ${systemBinding}`
-		);
 		const hasTrailingSemicolon = code.original.charCodeAt(this.end - 1) === 59; /*";"*/
-		if (systemBinding) {
+		const systemExportNames =
+			options.format === 'system' && options.exportNamesByVariable.get(this.variable);
+
+		if (systemExportNames) {
+			code.overwrite(
+				this.start,
+				declarationStart,
+				`${options.varOrConst} ${this.variable.getName()} = exports('${systemExportNames[0]}', `
+			);
 			code.appendRight(
 				hasTrailingSemicolon ? this.end - 1 : this.end,
 				')' + (hasTrailingSemicolon ? '' : ';')
 			);
-		} else if (!hasTrailingSemicolon) {
-			code.appendLeft(this.end, ';');
+		} else {
+			code.overwrite(
+				this.start,
+				declarationStart,
+				`${options.varOrConst} ${this.variable.getName()} = `
+			);
+			if (!hasTrailingSemicolon) {
+				code.appendLeft(this.end, ';');
+			}
 		}
 	}
 }

diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts
@@ -27,8 +27,11 @@ export default class Identifier extends NodeBase implements PatternNode {
 	variable: Variable | null = null;
 	private bound = false;
 
-	addExportedVariables(variables: Variable[]): void {
-		if (this.variable !== null && this.variable.exportName) {
+	addExportedVariables(
+		variables: Variable[],
+		exportNamesByVariable: Map<Variable, string[]>
+	): void {
+		if (this.variable !== null && exportNamesByVariable.has(this.variable)) {
 			variables.push(this.variable);
 		}
 	}

diff --git a/src/ast/nodes/ImportExpression.ts b/src/ast/nodes/ImportExpression.ts
@@ -66,7 +66,7 @@ export default class Import extends NodeBase {
 	renderFinalResolution(
 		code: MagicString,
 		resolution: string,
-		namespaceExportName: string | false,
+		namespaceExportName: string | false | undefined,
 		options: NormalizedOutputOptions
 	) {
 		code.overwrite(this.source.start, this.source.end, resolution);

diff --git a/src/ast/nodes/ObjectPattern.ts b/src/ast/nodes/ObjectPattern.ts
@@ -12,12 +12,18 @@ export default class ObjectPattern extends NodeBase implements PatternNode {
 	properties!: (Property | RestElement)[];
 	type!: NodeType.tObjectPattern;
 
-	addExportedVariables(variables: Variable[]): void {
+	addExportedVariables(
+		variables: Variable[],
+		exportNamesByVariable: Map<Variable, string[]>
+	): void {
 		for (const property of this.properties) {
 			if (property.type === NodeType.Property) {
-				((property.value as unknown) as PatternNode).addExportedVariables(variables);
+				((property.value as unknown) as PatternNode).addExportedVariables(
+					variables,
+					exportNamesByVariable
+				);
 			} else {
-				property.argument.addExportedVariables(variables);
+				property.argument.addExportedVariables(variables, exportNamesByVariable);
 			}
 		}
 	}