Merge pull request #62 from rollup/gh-59

Handle external dependencies correctly

src/Bundle.js

@@ -13,11 +13,11 @@ import getExportMode from './utils/getExportMode';
 import getIndentString from './utils/getIndentString';
 import { unixizePath } from './utils/normalizePlatform.js';
 
-function isEmptyExportedVarDeclaration ( node, module, allBundleExports ) {
+function isEmptyExportedVarDeclaration ( node, module, allBundleExports, es6 ) {
 	if ( node.type !== 'VariableDeclaration' || node.declarations[0].init ) return false;
 
 	const name = node.declarations[0].id.name;
-	const canonicalName = module.getCanonicalName( name );
+	const canonicalName = module.getCanonicalName( name, es6 );
 
 	return canonicalName in allBundleExports;
 }
@@ -91,11 +91,10 @@ export default class Bundle {
 			})
 			.then( () => {
 				this.statements = this.sort();
-				this.deconflict();
 			});
 	}
 
-	deconflict () {
+	deconflict ( es6 ) {
 		let definers = blank();
 		let conflicts = blank();
 
@@ -124,10 +123,10 @@ export default class Bundle {
 			// we need to ensure that the name chosen for the expression does
 			// not conflict
 			if ( statement.node.type === 'ExportDefaultDeclaration' ) {
-				const name = module.getCanonicalName( 'default' );
+				const name = module.getCanonicalName( 'default', es6 );
 
 				const isProxy = statement.node.declaration && statement.node.declaration.type === 'Identifier';
-				const shouldDeconflict = !isProxy || ( module.getCanonicalName( statement.node.declaration.name ) !== name );
+				const shouldDeconflict = !isProxy || ( module.getCanonicalName( statement.node.declaration.name, es6 ) !== name );
 
 				if ( shouldDeconflict && !~names.indexOf( name ) ) {
 					names.push( name );
@@ -218,6 +217,7 @@ export default class Bundle {
 		let magicString = new MagicString.Bundle({ separator: '' });
 
 		const format = options.format || 'es6';
+		this.deconflict( format === 'es6' );
 
 		// If we have named exports from the bundle, and those exports
 		// are assigned to *within* the bundle, we may need to rewrite e.g.
@@ -243,7 +243,7 @@ export default class Bundle {
 				const originalDeclaration = this.entryModule.findDeclaration( exportDeclaration.localName );
 
 				if ( originalDeclaration && originalDeclaration.type === 'VariableDeclaration' ) {
-					const canonicalName = this.entryModule.getCanonicalName( exportDeclaration.localName );
+					const canonicalName = this.entryModule.getCanonicalName( exportDeclaration.localName, false );
 
 					allBundleExports[ canonicalName ] = `exports.${key}`;
 					this.varExports[ key ] = true;
@@ -268,12 +268,12 @@ export default class Bundle {
 				if ( statement.node.specifiers.length ) return;
 
 				// skip `export var foo;` if foo is exported
-				if ( isEmptyExportedVarDeclaration( statement.node.declaration, statement.module, allBundleExports ) ) return;
+				if ( isEmptyExportedVarDeclaration( statement.node.declaration, statement.module, allBundleExports, format === 'es6' ) ) return;
 			}
 
 			// skip empty var declarations for exported bindings
 			// (otherwise we're left with `exports.foo;`, which is useless)
-			if ( isEmptyExportedVarDeclaration( statement.node, statement.module, allBundleExports ) ) return;
+			if ( isEmptyExportedVarDeclaration( statement.node, statement.module, allBundleExports, format === 'es6' ) ) return;
 
 			let replacements = blank();
 			let bundleExports = blank();
@@ -281,7 +281,7 @@ export default class Bundle {
 			keys( statement.dependsOn )
 				.concat( keys( statement.defines ) )
 				.forEach( name => {
-					const canonicalName = statement.module.getCanonicalName( name );
+					const canonicalName = statement.module.getCanonicalName( name, format === 'es6' );
 
 					if ( allBundleExports[ canonicalName ] ) {
 						bundleExports[ name ] = replacements[ name ] = allBundleExports[ canonicalName ];
@@ -307,9 +307,9 @@ export default class Bundle {
 
 				else if ( statement.node.type === 'ExportDefaultDeclaration' ) {
 					const module = statement.module;
-					const canonicalName = module.getCanonicalName( 'default' );
+					const canonicalName = module.getCanonicalName( 'default', format === 'es6' );
 
-					if ( statement.node.declaration.type === 'Identifier' && canonicalName === module.getCanonicalName( statement.node.declaration.name ) ) {
+					if ( statement.node.declaration.type === 'Identifier' && canonicalName === module.getCanonicalName( statement.node.declaration.name, format === 'es6' ) ) {
 						return;
 					}
 
@@ -370,8 +370,8 @@ export default class Bundle {
 		const namespaceBlock = this.internalNamespaceModules.map( module => {
 			const exportKeys = keys( module.exports );
 
-			return `var ${module.getCanonicalName('*')} = {\n` +
-				exportKeys.map( key => `${indentString}get ${key} () { return ${module.getCanonicalName(key)}; }` ).join( ',\n' ) +
+			return `var ${module.getCanonicalName('*', format === 'es6')} = {\n` +
+				exportKeys.map( key => `${indentString}get ${key} () { return ${module.getCanonicalName(key, format === 'es6')}; }` ).join( ',\n' ) +
 			`\n};\n\n`;
 		}).join( '' );
 

src/ExternalModule.js

@@ -12,24 +12,30 @@ export default class ExternalModule {
 		this.suggestedNames = blank();
 
 		this.needsDefault = false;
+
+		// Invariant: needsNamed and needsAll are never both true at once.
+		// Because an import with both a namespace and named import is invalid:
+		//
+		// 		import * as ns, { a } from '...'
+		//
 		this.needsNamed = false;
+		this.needsAll = false;
 	}
 
 	findDefiningStatement () {
 		return null;
 	}
 
-	getCanonicalName ( name ) {
+	getCanonicalName ( name, es6 ) {
 		if ( name === 'default' ) {
-			return this.needsNamed ? `${this.name}__default` : this.name;
+			return this.needsNamed && !es6 ? `${this.name}__default` : this.name;
 		}
 
 		if ( name === '*' ) {
-			return this.name;
+			return this.name; // TODO is this correct in ES6?
 		}
 
-		// TODO this depends on the output format... works for CJS etc but not ES6
-		return `${this.name}.${name}`;
+		return es6 ? ( this.canonicalNames[ name ] || name ) : `${this.name}.${name}`;
 	}
 
 	rename ( name, replacement ) {

src/Module.js

@@ -281,7 +281,7 @@ export default class Module {
 		return null;
 	}
 
-	getCanonicalName ( localName ) {
+	getCanonicalName ( localName, es6 ) {
 		// Special case
 		if ( localName === 'default' && ( this.exports.default.isModified || !this.suggestedNames.default ) ) {
 			let canonicalName = makeLegalIdentifier( this.id.replace( dirname( this.bundle.entryModule.id ) + '/', '' ).replace( /\.js$/, '' ) );
@@ -292,7 +292,9 @@ export default class Module {
 			localName = this.suggestedNames[ localName ];
 		}
 
-		if ( !this.canonicalNames[ localName ] ) {
+		const id = localName + ( es6 ? '-es6' : '' ); // TODO ugh this seems like a terrible hack
+
+		if ( !this.canonicalNames[ id ] ) {
 			let canonicalName;
 
 			if ( this.imports[ localName ] ) {
@@ -317,7 +319,7 @@ export default class Module {
 						}
 					}
 
-					canonicalName = module.getCanonicalName( exporterLocalName );
+					canonicalName = module.getCanonicalName( exporterLocalName, es6 );
 				}
 			}
 
@@ -325,10 +327,10 @@ export default class Module {
 				canonicalName = localName;
 			}
 
-			this.canonicalNames[ localName ] = canonicalName;
+			this.canonicalNames[ id ] = canonicalName;
 		}
 
-		return this.canonicalNames[ localName ];
+		return this.canonicalNames[ id ];
 	}
 
 	mark ( name ) {
@@ -369,6 +371,8 @@ export default class Module {
 					if ( module.isExternal ) {
 						if ( importDeclaration.name === 'default' ) {
 							module.needsDefault = true;
+						} else if ( importDeclaration.name === '*' ) {
+							module.needsAll = true;
 						} else {
 							module.needsNamed = true;
 						}
@@ -561,7 +565,8 @@ export default class Module {
 	}
 
 	rename ( name, replacement ) {
-		this.canonicalNames[ name ] = replacement;
+		// TODO again, hacky...
+		this.canonicalNames[ name ] = this.canonicalNames[ name + '-es6' ] = replacement;
 	}
 
 	suggestName ( defaultOrBatch, suggestion ) {

src/finalisers/amd.js

@@ -1,4 +1,5 @@
 import { getName, quoteId } from '../utils/map-helpers';
+import getInteropBlock from './shared/getInteropBlock';
 
 export default function amd ( bundle, magicString, { exportMode, indentString }, options ) {
 	let deps = bundle.externalModules.map( quoteId );
@@ -15,6 +16,10 @@ export default function amd ( bundle, magicString, { exportMode, indentString },
 
 	const intro = `define(${params}function (${args.join( ', ' )}) { 'use strict';\n\n`;
 
+	// var foo__default = 'default' in foo ? foo['default'] : foo;
+	const interopBlock = getInteropBlock( bundle );
+	if ( interopBlock ) magicString.prepend( interopBlock + '\n\n' );
+
 	const exports = bundle.entryModule.exports;
 
 	let exportBlock;
@@ -27,9 +32,7 @@ export default function amd ( bundle, magicString, { exportMode, indentString },
 		}).join( '\n' );
 	}
 
-	if ( exportBlock ) {
-		magicString.append( '\n\n' + exportBlock );
-	}
+	if ( exportBlock ) magicString.append( '\n\n' + exportBlock );
 
 	return magicString
 		.indent( indentString )

src/finalisers/es6.js

@@ -1,7 +1,37 @@
 import { keys } from '../utils/object';
 
 export default function es6 ( bundle, magicString, { exportMode }, options ) {
-	const introBlock = ''; // TODO...
+	const importBlock = bundle.externalModules
+		.map( module => {
+			const specifiers = [];
+
+			if ( module.needsDefault ) {
+				specifiers.push( module.importedByBundle.filter( declaration =>
+					declaration.name === 'default' )[0].localName );
+			}
+
+			if ( module.needsAll ) {
+				specifiers.push( '* as ' + module.importedByBundle.filter( declaration =>
+					declaration.name === '*' )[0].localName );
+			}
+
+			if ( module.needsNamed ) {
+				specifiers.push( '{ ' + module.importedByBundle
+					.filter( declaration => !/^(default|\*)$/.test( declaration.name ) )
+					.map( ({ name, localName }) =>
+						name === localName ? name : `${name} as ${localName}` )
+					.join( ', ' ) + ' }' );
+			}
+
+			return specifiers.length ?
+				`import ${specifiers.join( ', ' )} from '${module.id}';` :
+				`import '${module.id}';`;
+		})
+		.join( '\n' );
+
+	if ( importBlock ) {
+		magicString.prepend( importBlock + '\n\n' );
+	}
 
 	const exports = bundle.entryModule.exports;
 	const exportBlock = keys( exports ).map( exportedName => {

src/finalisers/iife.js

@@ -1,5 +1,6 @@
 import { blank } from '../utils/object';
 import { getName } from '../utils/map-helpers';
+import getInteropBlock from './shared/getInteropBlock';
 
 export default function iife ( bundle, magicString, { exportMode, indentString }, options ) {
 	const globalNames = options.globals || blank();
@@ -22,6 +23,10 @@ export default function iife ( bundle, magicString, { exportMode, indentString }
 	let intro = `(function (${args}) { 'use strict';\n\n`;
 	let outro = `\n\n})(${dependencies});`;
 
+	// var foo__default = 'default' in foo ? foo['default'] : foo;
+	const interopBlock = getInteropBlock( bundle );
+	if ( interopBlock ) magicString.prepend( interopBlock + '\n\n' );
+
 	if ( exportMode === 'default' ) {
 		intro = `var ${options.moduleName} = ${intro}`;
 		magicString.append( `\n\nreturn ${bundle.entryModule.getCanonicalName('default')};` );

src/finalisers/shared/getInteropBlock.js

@@ -0,0 +1,6 @@
+export default function getInteropBlock ( bundle ) {
+	return bundle.externalModules
+		.filter( module => module.needsDefault && module.needsNamed )
+		.map( module => `var ${module.name}__default = 'default' in ${module.name} ? ${module.name}['default'] : ${module.name};` )
+		.join( '\n' );
+}

src/finalisers/umd.js

@@ -1,5 +1,6 @@
 import { blank } from '../utils/object';
 import { getName, quoteId, req } from '../utils/map-helpers';
+import getInteropBlock from './shared/getInteropBlock';
 
 export default function umd ( bundle, magicString, { exportMode, indentString }, options ) {
 	if ( exportMode !== 'none' && !options.moduleName ) {
@@ -40,6 +41,10 @@ export default function umd ( bundle, magicString, { exportMode, indentString },
 
 		`.replace( /^\t\t/gm, '' ).replace( /^\t/gm, magicString.getIndentString() );
 
+	// var foo__default = 'default' in foo ? foo['default'] : foo;
+	const interopBlock = getInteropBlock( bundle );
+	if ( interopBlock ) magicString.prepend( interopBlock + '\n\n' );
+
 	const exports = bundle.entryModule.exports;
 
 	let exportBlock;

test/form/external-imports/_config.js

@@ -1,6 +1,6 @@
 module.exports = {
 	description: 'prefixes global names with `global.` when creating UMD bundle (#57)',
 	options: {
-		external: [ 'factory' ]
+		external: [ 'factory', 'baz', 'shipping-port', 'alphabet' ]
 	}
 };

test/form/external-imports/_expected/amd.js

@@ -1,5 +1,11 @@
-define(['factory'], function (factory) { 'use strict';
+define(['factory', 'baz', 'shipping-port', 'alphabet'], function (factory, baz, containers, alphabet) { 'use strict';
+
+	var alphabet__default = 'default' in alphabet ? alphabet['default'] : alphabet;
 
 	factory( null );
+	baz.foo( baz.bar );
+	containers.forEach( console.log, console );
+	console.log( alphabet.a );
+	console.log( alphabet__default.length );
 
 });

test/form/external-imports/_expected/cjs.js

@@ -2,5 +2,13 @@
 
 var factory = require('factory');
 factory = 'default' in factory ? factory['default'] : factory;
+var baz = require('baz');
+var containers = require('shipping-port');
+var alphabet = require('alphabet');
+var alphabet__default = 'default' in alphabet ? alphabet['default'] : alphabet;
 
 factory( null );
+baz.foo( baz.bar );
+containers.forEach( console.log, console );
+console.log( alphabet.a );
+console.log( alphabet__default.length );

test/form/external-imports/_expected/es6.js

@@ -1 +1,10 @@
+import factory from 'factory';
+import { bar, foo } from 'baz';
+import * as containers from 'shipping-port';
+import alphabet, { a } from 'alphabet';
+
 factory( null );
+foo( bar );
+containers.forEach( console.log, console );
+console.log( a );
+console.log( alphabet.length );

test/form/external-imports/_expected/iife.js

@@ -1,5 +1,11 @@
-(function (factory) { 'use strict';
+(function (factory,baz,containers,alphabet) { 'use strict';
+
+	var alphabet__default = 'default' in alphabet ? alphabet['default'] : alphabet;
 
 	factory( null );
+	baz.foo( baz.bar );
+	containers.forEach( console.log, console );
+	console.log( alphabet.a );
+	console.log( alphabet__default.length );
 
-})(factory);
+})(factory,baz,containers,alphabet);

test/form/external-imports/_expected/umd.js

@@ -1,9 +1,15 @@
 (function (global, factory) {
-	typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('factory')) :
-	typeof define === 'function' && define.amd ? define(['factory'], factory) :
-	factory(global.factory);
-}(this, function (factory) { 'use strict';
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('factory'), require('baz'), require('shipping-port'), require('alphabet')) :
+	typeof define === 'function' && define.amd ? define(['factory', 'baz', 'shipping-port', 'alphabet'], factory) :
+	factory(global.factory,global.baz,global.containers,global.alphabet);
+}(this, function (factory,baz,containers,alphabet) { 'use strict';
+
+	var alphabet__default = 'default' in alphabet ? alphabet['default'] : alphabet;
 
 	factory( null );
+	baz.foo( baz.bar );
+	containers.forEach( console.log, console );
+	console.log( alphabet.a );
+	console.log( alphabet__default.length );
 
 }));

test/form/external-imports/main.js

@@ -1,2 +1,10 @@
 import factory from 'factory';
+import { foo, bar } from 'baz';
+import * as containers from 'shipping-port';
+import alphabet, { a, b } from 'alphabet';
+
 factory( null );
+foo( bar );
+containers.forEach( console.log, console );
+console.log( a );
+console.log( alphabet.length );