Merge pull request #107 from rollup/gh-84

deconflict against globals

src/transform.js

@@ -16,11 +16,11 @@ const firstpassGlobal = /\b(?:require|module|exports|global)\b/;
 const firstpassNoGlobal = /\b(?:require|module|exports)\b/;
 const importExportDeclaration = /^(?:Import|Export(?:Named|Default))Declaration/;
 
-function deconflict ( scope, identifier ) {
+function deconflict ( scope, globals, identifier ) {
 	let i = 1;
 	let deconflicted = identifier;
 
-	while ( scope.contains( deconflicted ) ) deconflicted = `${identifier}_${i++}`;
+	while ( scope.contains( deconflicted ) || globals.has( deconflicted ) ) deconflicted = `${identifier}_${i++}`;
 	scope.declarations[ deconflicted ] = true;
 
 	return deconflicted;
@@ -64,7 +64,9 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 	let lexicalDepth = 0;
 	let programDepth = 0;
 
-	const HELPERS_NAME = deconflict( scope, 'commonjsHelpers' );
+	const globals = new Set();
+
+	const HELPERS_NAME = deconflict( scope, globals, 'commonjsHelpers' ); // TODO technically wrong since globals isn't populated yet, but ¯\_(ツ)_/¯
 
 	const namedExports = {};
 	if ( customNamedExports ) customNamedExports.forEach( name => namedExports[ name ] = true );
@@ -135,18 +137,23 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 			}
 
 			if ( node.type === 'Identifier' ) {
-				if ( ( node.name in uses ) && isReference( node, parent ) && !scope.contains( node.name ) ) {
-					uses[ node.name ] = true;
-					if ( node.name === 'global' && !ignoreGlobal ) {
-						magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
+				if ( isReference( node, parent ) && !scope.contains( node.name ) ) {
+					if ( node.name in uses ) {
+						uses[ node.name ] = true;
+						if ( node.name === 'global' && !ignoreGlobal ) {
+							magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
+						}
+
+						// if module or exports are used outside the context of an assignment
+						// expression, we need to wrap the module
+						if ( node.name === 'module' || node.name === 'exports' ) {
+							shouldWrap = true;
+						}
 					}
 
-					// if module or exports are used outside the context of an assignment
-					// expression, we need to wrap the module
-					if ( node.name === 'module' || node.name === 'exports' ) {
-						shouldWrap = true;
-					}
+					globals.add( node.name );
 				}
+
 				return;
 			}
 
@@ -215,7 +222,7 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 	let wrapperStart = '';
 	let wrapperEnd = '';
 
-	const moduleName = deconflict( scope, getName( id ) );
+	const moduleName = deconflict( scope, globals, getName( id ) );
 	if ( !isEntry ) {
 		const exportModuleExports = `export { ${moduleName} as __moduleExports };`;
 		namedExportDeclarations.push( exportModuleExports );
@@ -235,7 +242,7 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 				let declaration;
 
 				if ( x === name ) {
-					const deconflicted = deconflict( scope, name );
+					const deconflicted = deconflict( scope, globals, name );
 					declaration = `var ${deconflicted} = ${moduleName}.${x};\nexport { ${deconflicted} as ${x} };`;
 				} else {
 					declaration = `export var ${x} = ${moduleName}.${x};`;
@@ -262,7 +269,7 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 					magicString.overwrite( node.start, right.start, `var ${moduleName} = ` );
 				} else {
 					const name = match[1];
-					const deconflicted = deconflict( scope, name );
+					const deconflicted = deconflict( scope, globals, name );
 
 					names.push({ name, deconflicted });
 

test/function/assumed-globals/_config.js

@@ -0,0 +1,5 @@
+module.exports = {
+	context: {
+		document: { real: true }
+	}
+};

test/function/assumed-globals/document.js

@@ -0,0 +1,5 @@
+if ( typeof document !== 'undefined' ) {
+	module.exports = document;
+} else {
+	module.exports = { fake: true };
+}

test/function/assumed-globals/main.js

@@ -0,0 +1,3 @@
+import document from './document.js';
+
+assert.deepEqual( document, { real: true });

test/test.js

@@ -11,11 +11,15 @@ require( 'source-map-support' ).install();
 
 process.chdir( __dirname );
 
-function execute ( code ) {
+function execute ( code, context = {} ) {
 	let fn;
 
+	const contextKeys = Object.keys( context );
+
+	const argNames = contextKeys.concat( 'module', 'exports', 'require', 'global', 'assert', code );
+
 	try {
-		fn = new Function( 'module', 'exports', 'require', 'global', 'assert', code );
+		fn = new Function( ...argNames );
 	} catch ( err ) {
 		// syntax error
 		console.log( code );
@@ -25,7 +29,15 @@ function execute ( code ) {
 	const module = { exports: {} };
 	const global = {};
 
-	fn( module, module.exports, name => name, global, assert );
+	const argValues = contextKeys.map( key => context[ key ] ).concat(
+		module,
+		module.exports,
+		name => name,
+		global,
+		assert
+	);
+
+	fn( ...argValues );
 
 	return {
 		code,
@@ -34,9 +46,9 @@ function execute ( code ) {
 	};
 }
 
-function executeBundle ( bundle ) {
+function executeBundle ( bundle, context ) {
 	const { code } = bundle.generate({ format: 'cjs' });
-	return execute( code );
+	return execute( code, context );
 }
 
 describe( 'rollup-plugin-commonjs', () => {
@@ -84,7 +96,7 @@ describe( 'rollup-plugin-commonjs', () => {
 						console.error( code );
 					}
 
-					const { exports, global } = execute( code );
+					const { exports, global } = execute( code, config.context );
 
 					if ( config.exports ) config.exports( exports );
 					if ( config.global ) config.global( global );