Merge pull request #1224 from rollup/gh-1217

omit globals for empty imports

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({

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;
 
+		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 fallback;
 	};
 }

src/finalisers/shared/propertyStringFor.js

@@ -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;

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( '' );
+}

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 [];
+}

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'` );

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 );
+		}
+	}
+};

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;
+
+});

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;

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;

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));

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;
+
+})));

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();

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 );
+		}
+	}
+};

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;
+
+});

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;

test/form/external-empty-import-no-global/_expected/es.js

@@ -0,0 +1,5 @@
+import 'babel-polyfill';
+
+var main = new WeakMap();
+
+export default main;

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;
+
+}());

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;
+
+})));

test/form/external-empty-import-no-global/main.js

@@ -0,0 +1,3 @@
+import 'babel-polyfill';
+
+export default new WeakMap();