Deduplicate imports referencing default exports and their original va…

…riables (#2719)

* Deduplicate imports referencing default exports and their original variables

* Update changelog

CHANGELOG.md

@@ -1,5 +1,14 @@
 # rollup changelog
 
+## 1.2.4
+*2019-02-25*
+
+### Bug Fixes
+* Fix an issue where a variable was imported twice under the same name (#2715)
+
+### Pull Requests
+* [#2715](https://github.com/rollup/rollup/pull/2715): Deduplicate imports referencing default exports and their original variables (@lukastaegert)
+
 ## 1.2.3
 *2019-02-23*
 

src/Chunk.ts

@@ -2,7 +2,9 @@ import sha256 from 'hash.js/lib/hash/sha/256';
 import MagicString, { Bundle as MagicStringBundle, SourceMap } from 'magic-string';
 import * as NodeType from './ast/nodes/NodeType';
 import { UNDEFINED_EXPRESSION } from './ast/values';
-import { isExportDefaultVariable } from './ast/variables/ExportDefaultVariable';
+import ExportDefaultVariable, {
+	isExportDefaultVariable
+} from './ast/variables/ExportDefaultVariable';
 import ExportShimVariable from './ast/variables/ExportShimVariable';
 import GlobalVariable from './ast/variables/GlobalVariable';
 import LocalVariable from './ast/variables/LocalVariable';
@@ -478,17 +480,22 @@ export default class Chunk {
 			exportDeclaration.push({ imported: importName, reexported: exportName });
 		}
 
+		const importsAsArray = Array.from(this.imports);
 		const dependencies: ChunkDependencies = [];
 
-		this.dependencies.forEach(dep => {
-			const importsFromDependency = Array.from(this.imports).filter(variable =>
-				variable.module instanceof Module ? variable.module.chunk === dep : variable.module === dep
-			);
-
-			let imports: ImportSpecifier[];
-			if (importsFromDependency.length) {
-				imports = [];
-				for (const variable of importsFromDependency) {
+		for (const dep of this.dependencies) {
+			const imports: ImportSpecifier[] = [];
+			for (const variable of importsAsArray) {
+				if (
+					(variable.module instanceof Module
+						? variable.module.chunk === dep
+						: variable.module === dep) &&
+					!(
+						variable instanceof ExportDefaultVariable &&
+						variable.referencesOriginal() &&
+						this.imports.has(variable.getOriginalVariable())
+					)
+				) {
 					const local = variable.getName();
 					const imported =
 						variable.module instanceof ExternalModule
@@ -534,9 +541,9 @@ export default class Chunk {
 				exportsNames,
 				exportsDefault,
 				reexports,
-				imports
+				imports: imports.length > 0 ? imports : null
 			});
-		});
+		}
 
 		return dependencies;
 	}

src/ast/variables/ExportDefaultVariable.ts

@@ -47,6 +47,10 @@ export default class ExportDefaultVariable extends LocalVariable {
 		return this.referencesOriginal() ? this.originalId.variable.getName() : super.getName();
 	}
 
+	getOriginalVariable(): Variable | null {
+		return (this.originalId && this.originalId.variable) || null;
+	}
+
 	getOriginalVariableName(): string | null {
 		return (this.originalId && this.originalId.name) || null;
 	}

test/chunking-form/samples/deduplicate-imports-referencing-originals/_config.js

@@ -0,0 +1,7 @@
+module.exports = {
+	description:
+		'do not import variables that reference an original if the original is already imported',
+	options: {
+		input: ['main1', 'main2']
+	}
+};

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/amd/generated-chunk.js

@@ -0,0 +1,8 @@
+define(['exports'], function (exports) { 'use strict';
+
+	const foo = {};
+
+	exports.foo = foo;
+	exports.bar = foo;
+
+});

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/amd/main1.js

@@ -0,0 +1,5 @@
+define(['./generated-chunk.js'], function (__chunk_1) { 'use strict';
+
+	console.log(__chunk_1.bar, __chunk_1.bar);
+
+});

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/amd/main2.js

@@ -0,0 +1,5 @@
+define(['./generated-chunk.js'], function (__chunk_1) { 'use strict';
+
+	console.log(__chunk_1.bar, __chunk_1.bar);
+
+});

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/cjs/generated-chunk.js

@@ -0,0 +1,6 @@
+'use strict';
+
+const foo = {};
+
+exports.foo = foo;
+exports.bar = foo;

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/cjs/main1.js

@@ -0,0 +1,5 @@
+'use strict';
+
+var __chunk_1 = require('./generated-chunk.js');
+
+console.log(__chunk_1.bar, __chunk_1.bar);

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/cjs/main2.js

@@ -0,0 +1,5 @@
+'use strict';
+
+var __chunk_1 = require('./generated-chunk.js');
+
+console.log(__chunk_1.bar, __chunk_1.bar);

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/es/generated-chunk.js

@@ -0,0 +1,3 @@
+const foo = {};
+
+export { foo as a, foo as b };

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/es/main1.js

@@ -0,0 +1,3 @@
+import { a as bar } from './generated-chunk.js';
+
+console.log(bar, bar);

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/es/main2.js

@@ -0,0 +1,3 @@
+import { a as bar } from './generated-chunk.js';
+
+console.log(bar, bar);

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/system/generated-chunk.js

@@ -0,0 +1,12 @@
+System.register([], function (exports, module) {
+	'use strict';
+	return {
+		execute: function () {
+
+			const foo = exports('a', {});
+
+			exports('b', foo);
+
+		}
+	};
+});

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/system/main1.js

@@ -0,0 +1,14 @@
+System.register(['./generated-chunk.js'], function (exports, module) {
+	'use strict';
+	var bar;
+	return {
+		setters: [function (module) {
+			bar = module.a;
+		}],
+		execute: function () {
+
+			console.log(bar, bar);
+
+		}
+	};
+});

test/chunking-form/samples/deduplicate-imports-referencing-originals/_expected/system/main2.js

@@ -0,0 +1,14 @@
+System.register(['./generated-chunk.js'], function (exports, module) {
+	'use strict';
+	var bar;
+	return {
+		setters: [function (module) {
+			bar = module.a;
+		}],
+		execute: function () {
+
+			console.log(bar, bar);
+
+		}
+	};
+});

test/chunking-form/samples/deduplicate-imports-referencing-originals/foo.js

@@ -0,0 +1 @@
+export const foo = {};

test/chunking-form/samples/deduplicate-imports-referencing-originals/main1.js

@@ -0,0 +1,4 @@
+import { foo } from './foo';
+import bar from './proxy';
+
+console.log(foo, bar);

test/chunking-form/samples/deduplicate-imports-referencing-originals/main2.js

@@ -0,0 +1,4 @@
+import { foo } from './foo';
+import bar from './proxy';
+
+console.log(foo, bar);

test/chunking-form/samples/deduplicate-imports-referencing-originals/proxy.js

@@ -0,0 +1,2 @@
+import { foo } from './foo';
+export default foo;