From 502e58d0b03dfd49ce781ba92b64702ee917bc0d Mon Sep 17 00:00:00 2001
From: Rich-Harris <richard.a.harris@gmail.com>
Date: Thu, 5 Jan 2017 12:24:33 -0500
Subject: [PATCH] omit globals for empty imports (#1217)

---
 src/finalisers/iife.js                        | 20 ++++++++--------
 src/finalisers/shared/getGlobalNameMaker.js   | 16 ++++++++-----
 src/finalisers/shared/propertyStringFor.js    | 18 ---------------
 src/finalisers/shared/sanitize.js             | 11 +++++++++
 src/finalisers/shared/trimEmptyImports.js     | 12 ++++++++++
 src/finalisers/umd.js                         | 23 ++++++++++---------
 .../_config.js                                | 11 +++++++++
 .../_expected/amd.js                          |  9 ++++++++
 .../_expected/cjs.js                          | 10 ++++++++
 .../_expected/es.js                           |  8 +++++++
 .../_expected/iife.js                         | 10 ++++++++
 .../_expected/umd.js                          | 13 +++++++++++
 .../external-empty-import-no-global-b/main.js |  6 +++++
 .../_config.js                                | 10 ++++++++
 .../_expected/amd.js                          |  7 ++++++
 .../_expected/cjs.js                          |  7 ++++++
 .../_expected/es.js                           |  5 ++++
 .../_expected/iife.js                         |  8 +++++++
 .../_expected/umd.js                          | 11 +++++++++
 .../external-empty-import-no-global/main.js   |  3 +++
 20 files changed, 172 insertions(+), 46 deletions(-)
 delete mode 100644 src/finalisers/shared/propertyStringFor.js
 create mode 100644 src/finalisers/shared/sanitize.js
 create mode 100644 src/finalisers/shared/trimEmptyImports.js
 create mode 100644 test/form/external-empty-import-no-global-b/_config.js
 create mode 100644 test/form/external-empty-import-no-global-b/_expected/amd.js
 create mode 100644 test/form/external-empty-import-no-global-b/_expected/cjs.js
 create mode 100644 test/form/external-empty-import-no-global-b/_expected/es.js
 create mode 100644 test/form/external-empty-import-no-global-b/_expected/iife.js
 create mode 100644 test/form/external-empty-import-no-global-b/_expected/umd.js
 create mode 100644 test/form/external-empty-import-no-global-b/main.js
 create mode 100644 test/form/external-empty-import-no-global/_config.js
 create mode 100644 test/form/external-empty-import-no-global/_expected/amd.js
 create mode 100644 test/form/external-empty-import-no-global/_expected/cjs.js
 create mode 100644 test/form/external-empty-import-no-global/_expected/es.js
 create mode 100644 test/form/external-empty-import-no-global/_expected/iife.js
 create mode 100644 test/form/external-empty-import-no-global/_expected/umd.js
 create mode 100644 test/form/external-empty-import-no-global/main.js

diff --git a/src/finalisers/iife.js b/src/finalisers/iife.js
index aad4491..f2c6902 100644
--- a/src/finalisers/iife.js
+++ b/src/finalisers/iife.js
@@ -4,14 +4,9 @@ import error from '../utils/error.js';
 import getInteropBlock from './shared/getInteropBlock.js';
 import getExportBlock from './shared/getExportBlock.js';
 import getGlobalNameMaker from './shared/getGlobalNameMaker.js';
-import propertyStringFor from './shared/propertyStringFor';
+import { property, keypath } from './shared/sanitize.js';
 import warnOnBuiltins from './shared/warnOnBuiltins.js';
-
-// thisProp('foo.bar-baz.qux') === "this.foo['bar-baz'].qux"
-const thisProp = propertyStringFor('this');
-
-// propString('foo.bar-baz.qux') === ".foo['bar-baz'].qux"
-const propString = propertyStringFor('');
+import trimEmptyImports from './shared/trimEmptyImports.js';
 
 function setupNamespace ( keypath ) {
 	const parts = keypath.split( '.' );
@@ -21,20 +16,23 @@ function setupNamespace ( keypath ) {
 	let acc = 'this';
 
 	return parts
-		.map( part => ( acc += propString(part), `${acc} = ${acc} || {};` ) )
+		.map( part => ( acc += property( part ), `${acc} = ${acc} || {};` ) )
 		.join( '\n' ) + '\n';
 }
 
+const thisProp = name => `this${keypath( name )}`;
+
 export default function iife ( bundle, magicString, { exportMode, indentString, intro, outro }, options ) {
-	const globalNameMaker = getGlobalNameMaker( options.globals || blank(), bundle );
+	const globalNameMaker = getGlobalNameMaker( options.globals || blank(), bundle, 'null' );
 
 	const name = options.moduleName;
 	const isNamespaced = name && ~name.indexOf( '.' );
 
 	warnOnBuiltins( bundle );
 
-	const dependencies = bundle.externalModules.map( globalNameMaker );
-	const args = bundle.externalModules.map( getName );
+	const external = trimEmptyImports( bundle.externalModules );
+	const dependencies = external.map( globalNameMaker );
+	const args = external.map( getName );
 
 	if ( exportMode !== 'none' && !name ) {
 		error({
diff --git a/src/finalisers/shared/getGlobalNameMaker.js b/src/finalisers/shared/getGlobalNameMaker.js
index 7a1d622..ee34756 100644
--- a/src/finalisers/shared/getGlobalNameMaker.js
+++ b/src/finalisers/shared/getGlobalNameMaker.js
@@ -1,15 +1,19 @@
-export default function getGlobalNameMaker ( globals, bundle ) {
+export default function getGlobalNameMaker ( globals, bundle, fallback = null ) {
 	const fn = typeof globals === 'function' ? globals : id => globals[ id ];
 
 	return function ( module ) {
 		const name = fn( module.id );
 		if ( name ) return name;
 
-		bundle.warn({
-			code: 'MISSING_GLOBAL_NAME',
-			message: `No name was provided for external module '${module.id}' in options.globals – guessing '${module.name}'`
-		});
+		if ( Object.keys( module.declarations ).length > 0 ) {
+			bundle.warn({
+				code: 'MISSING_GLOBAL_NAME',
+				message: `No name was provided for external module '${module.id}' in options.globals – guessing '${module.name}'`
+			});
 
-		return module.name;
+			return module.name;
+		}
+
+		return fallback;
 	};
 }
diff --git a/src/finalisers/shared/propertyStringFor.js b/src/finalisers/shared/propertyStringFor.js
deleted file mode 100644
index 72ad1bb..0000000
--- a/src/finalisers/shared/propertyStringFor.js
+++ /dev/null
@@ -1,18 +0,0 @@
-// Generate strings which dereference dotted properties, but use array notation `['prop-deref']`
-// if the property name isn't trivial
-
-const shouldUseDot = /^[a-zA-Z$_][a-zA-Z0-9$_]*$/;
-const dereferenceString = prop =>
-    prop.match(shouldUseDot) ? `.${prop}` : `['${prop}']`;
-
-/**
- * returns a function which generates property dereference strings for the given name
- *
- * const getGlobalProp = propertyStringFor('global');
- * getGlobalProp('foo.bar-baz.qux') => `global.bar['bar-baz'].qux`
- */
-const propertyStringFor = objName => propName =>
-  objName + propName.split('.').map(dereferenceString).join('');
-
-
-export default propertyStringFor;
\ No newline at end of file
diff --git a/src/finalisers/shared/sanitize.js b/src/finalisers/shared/sanitize.js
new file mode 100644
index 0000000..e158976
--- /dev/null
+++ b/src/finalisers/shared/sanitize.js
@@ -0,0 +1,11 @@
+// Generate strings which dereference dotted properties, but use array notation `['prop-deref']`
+// if the property name isn't trivial
+const shouldUseDot = /^[a-zA-Z$_][a-zA-Z0-9$_]*$/;
+
+export function property ( prop ) {
+	return shouldUseDot.test( prop ) ? `.${prop}` : `['${prop}']`;
+}
+
+export function keypath ( keypath ) {
+	return keypath.split( '.' ).map( property ).join( '' );
+}
diff --git a/src/finalisers/shared/trimEmptyImports.js b/src/finalisers/shared/trimEmptyImports.js
new file mode 100644
index 0000000..e31bcc9
--- /dev/null
+++ b/src/finalisers/shared/trimEmptyImports.js
@@ -0,0 +1,12 @@
+export default function trimEmptyImports ( modules ) {
+	let i = modules.length;
+
+	while ( i-- ) {
+		const module = modules[i];
+		if ( Object.keys( module.declarations ).length > 0 ) {
+			return modules.slice( 0, i + 1 );
+		}
+	}
+
+	return [];
+}
diff --git a/src/finalisers/umd.js b/src/finalisers/umd.js
index b624307..5102e2b 100644
--- a/src/finalisers/umd.js
+++ b/src/finalisers/umd.js
@@ -5,23 +5,23 @@ import getInteropBlock from './shared/getInteropBlock.js';
 import getExportBlock from './shared/getExportBlock.js';
 import getGlobalNameMaker from './shared/getGlobalNameMaker.js';
 import esModuleExport from './shared/esModuleExport.js';
-import propertyStringFor from './shared/propertyStringFor.js';
+import { property, keypath } from './shared/sanitize.js';
 import warnOnBuiltins from './shared/warnOnBuiltins.js';
+import trimEmptyImports from './shared/trimEmptyImports.js';
 
-// globalProp('foo.bar-baz') === "global.foo['bar-baz']"
-const globalProp = propertyStringFor('global');
-
-// propString('foo.bar-baz') === ".foo['bar']"
-const propString = propertyStringFor('');
+function globalProp ( name ) {
+	if ( !name ) return 'null';
+	return `global${ keypath( name ) }`;
+}
 
 function setupNamespace ( name ) {
 	const parts = name.split( '.' );
-	parts.pop();
+	const last = property( parts.pop() );
 
 	let acc = 'global';
 	return parts
-		.map( part => ( acc += propString(part), `${acc} = ${acc} || {}` ) )
-		.concat( globalProp(name) )
+		.map( part => ( acc += property( part ), `${acc} = ${acc} || {}` ) )
+		.concat( `${acc}${last}` )
 		.join( ', ' );
 }
 
@@ -41,9 +41,10 @@ export default function umd ( bundle, magicString, { exportMode, indentString, i
 
 	const amdDeps = bundle.externalModules.map( quotePath );
 	const cjsDeps = bundle.externalModules.map( req );
-	const globalDeps = bundle.externalModules.map( module => globalProp(globalNameMaker( module )) );
 
-	const args = bundle.externalModules.map( getName );
+	const trimmed = trimEmptyImports( bundle.externalModules );
+	const globalDeps = trimmed.map( module => globalProp( globalNameMaker( module ) ) );
+	const args = trimmed.map( getName );
 
 	if ( exportMode === 'named' ) {
 		amdDeps.unshift( `'exports'` );
diff --git a/test/form/external-empty-import-no-global-b/_config.js b/test/form/external-empty-import-no-global-b/_config.js
new file mode 100644
index 0000000..ef5dbea
--- /dev/null
+++ b/test/form/external-empty-import-no-global-b/_config.js
@@ -0,0 +1,11 @@
+module.exports = {
+	description: 'does not expect a global to be provided for empty imports (#1217)',
+	options: {
+		external: [ 'babel-polyfill', 'other' ],
+		moduleName: 'myBundle',
+		globals: { other: 'other' },
+		onwarn ( warning ) {
+			throw new Error( warning.message );
+		}
+	}
+};
diff --git a/test/form/external-empty-import-no-global-b/_expected/amd.js b/test/form/external-empty-import-no-global-b/_expected/amd.js
new file mode 100644
index 0000000..1881f36
--- /dev/null
+++ b/test/form/external-empty-import-no-global-b/_expected/amd.js
@@ -0,0 +1,9 @@
+define(['babel-polyfill', 'other'], function (babelPolyfill, other) { 'use strict';
+
+	other.x();
+
+	var main = new WeakMap();
+
+	return main;
+
+});
diff --git a/test/form/external-empty-import-no-global-b/_expected/cjs.js b/test/form/external-empty-import-no-global-b/_expected/cjs.js
new file mode 100644
index 0000000..b2b634a
--- /dev/null
+++ b/test/form/external-empty-import-no-global-b/_expected/cjs.js
@@ -0,0 +1,10 @@
+'use strict';
+
+require('babel-polyfill');
+var other = require('other');
+
+other.x();
+
+var main = new WeakMap();
+
+module.exports = main;
diff --git a/test/form/external-empty-import-no-global-b/_expected/es.js b/test/form/external-empty-import-no-global-b/_expected/es.js
new file mode 100644
index 0000000..6c29fd8
--- /dev/null
+++ b/test/form/external-empty-import-no-global-b/_expected/es.js
@@ -0,0 +1,8 @@
+import 'babel-polyfill';
+import { x } from 'other';
+
+x();
+
+var main = new WeakMap();
+
+export default main;
diff --git a/test/form/external-empty-import-no-global-b/_expected/iife.js b/test/form/external-empty-import-no-global-b/_expected/iife.js
new file mode 100644
index 0000000..23f04a2
--- /dev/null
+++ b/test/form/external-empty-import-no-global-b/_expected/iife.js
@@ -0,0 +1,10 @@
+var myBundle = (function (babelPolyfill,other) {
+	'use strict';
+
+	other.x();
+
+	var main = new WeakMap();
+
+	return main;
+
+}(null,other));
diff --git a/test/form/external-empty-import-no-global-b/_expected/umd.js b/test/form/external-empty-import-no-global-b/_expected/umd.js
new file mode 100644
index 0000000..d428b3f
--- /dev/null
+++ b/test/form/external-empty-import-no-global-b/_expected/umd.js
@@ -0,0 +1,13 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory(require('babel-polyfill'), require('other')) :
+	typeof define === 'function' && define.amd ? define(['babel-polyfill', 'other'], factory) :
+	(global.myBundle = factory(null,global.other));
+}(this, (function (babelPolyfill,other) { 'use strict';
+
+	other.x();
+
+	var main = new WeakMap();
+
+	return main;
+
+})));
diff --git a/test/form/external-empty-import-no-global-b/main.js b/test/form/external-empty-import-no-global-b/main.js
new file mode 100644
index 0000000..f7e6783
--- /dev/null
+++ b/test/form/external-empty-import-no-global-b/main.js
@@ -0,0 +1,6 @@
+import 'babel-polyfill';
+import { x } from 'other';
+
+x();
+
+export default new WeakMap();
diff --git a/test/form/external-empty-import-no-global/_config.js b/test/form/external-empty-import-no-global/_config.js
new file mode 100644
index 0000000..ad4e223
--- /dev/null
+++ b/test/form/external-empty-import-no-global/_config.js
@@ -0,0 +1,10 @@
+module.exports = {
+	description: 'does not expect a global to be provided for empty imports (#1217)',
+	options: {
+		external: [ 'babel-polyfill' ],
+		moduleName: 'myBundle',
+		onwarn ( warning ) {
+			throw new Error( warning.message );
+		}
+	}
+};
diff --git a/test/form/external-empty-import-no-global/_expected/amd.js b/test/form/external-empty-import-no-global/_expected/amd.js
new file mode 100644
index 0000000..a54a828
--- /dev/null
+++ b/test/form/external-empty-import-no-global/_expected/amd.js
@@ -0,0 +1,7 @@
+define(['babel-polyfill'], function (babelPolyfill) { 'use strict';
+
+	var main = new WeakMap();
+
+	return main;
+
+});
diff --git a/test/form/external-empty-import-no-global/_expected/cjs.js b/test/form/external-empty-import-no-global/_expected/cjs.js
new file mode 100644
index 0000000..c5b39fd
--- /dev/null
+++ b/test/form/external-empty-import-no-global/_expected/cjs.js
@@ -0,0 +1,7 @@
+'use strict';
+
+require('babel-polyfill');
+
+var main = new WeakMap();
+
+module.exports = main;
diff --git a/test/form/external-empty-import-no-global/_expected/es.js b/test/form/external-empty-import-no-global/_expected/es.js
new file mode 100644
index 0000000..7ac0688
--- /dev/null
+++ b/test/form/external-empty-import-no-global/_expected/es.js
@@ -0,0 +1,5 @@
+import 'babel-polyfill';
+
+var main = new WeakMap();
+
+export default main;
diff --git a/test/form/external-empty-import-no-global/_expected/iife.js b/test/form/external-empty-import-no-global/_expected/iife.js
new file mode 100644
index 0000000..02b6db6
--- /dev/null
+++ b/test/form/external-empty-import-no-global/_expected/iife.js
@@ -0,0 +1,8 @@
+var myBundle = (function () {
+	'use strict';
+
+	var main = new WeakMap();
+
+	return main;
+
+}());
diff --git a/test/form/external-empty-import-no-global/_expected/umd.js b/test/form/external-empty-import-no-global/_expected/umd.js
new file mode 100644
index 0000000..48ad8df
--- /dev/null
+++ b/test/form/external-empty-import-no-global/_expected/umd.js
@@ -0,0 +1,11 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory(require('babel-polyfill')) :
+	typeof define === 'function' && define.amd ? define(['babel-polyfill'], factory) :
+	(global.myBundle = factory());
+}(this, (function () { 'use strict';
+
+	var main = new WeakMap();
+
+	return main;
+
+})));
diff --git a/test/form/external-empty-import-no-global/main.js b/test/form/external-empty-import-no-global/main.js
new file mode 100644
index 0000000..dc13b34
--- /dev/null
+++ b/test/form/external-empty-import-no-global/main.js
@@ -0,0 +1,3 @@
+import 'babel-polyfill';
+
+export default new WeakMap();