Merge pull request #106 from rollup/optimisation

Optimisation

src/transform.js

@@ -16,11 +16,13 @@ const firstpassGlobal = /\b(?:require|module|exports|global)\b/;
 const firstpassNoGlobal = /\b(?:require|module|exports)\b/;
 const importExportDeclaration = /^(?:Import|Export(?:Named|Default))Declaration/;
 
-function deconflict ( identifier, code ) {
+function deconflict ( scope, identifier ) {
 	let i = 1;
 	let deconflicted = identifier;
 
-	while ( ~code.indexOf( deconflicted ) ) deconflicted = `${identifier}_${i++}`;
+	while ( scope.contains( deconflicted ) ) deconflicted = `${identifier}_${i++}`;
+	scope.declarations[ deconflicted ] = true;
+
 	return deconflicted;
 }
 
@@ -40,9 +42,6 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 	const firstpass = ignoreGlobal ? firstpassNoGlobal : firstpassGlobal;
 	if ( !firstpass.test( code ) ) return null;
 
-	const namedExports = {};
-	if ( customNamedExports ) customNamedExports.forEach( name => namedExports[ name ] = true );
-
 	const ast = tryParse( code, id );
 
 	// if there are top-level import/export declarations, this is ES not CommonJS
@@ -62,25 +61,36 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 	let scope = attachScopes( ast, 'scope' );
 	const uses = { module: false, exports: false, global: false };
 
-	let scopeDepth = 0;
+	let lexicalDepth = 0;
+	let programDepth = 0;
+
+	const HELPERS_NAME = deconflict( scope, 'commonjsHelpers' );
+
+	const namedExports = {};
+	if ( customNamedExports ) customNamedExports.forEach( name => namedExports[ name ] = true );
 
-	const HELPERS_NAME = deconflict( 'commonjsHelpers', code );
+	// TODO handle transpiled modules
+	let shouldWrap = /__esModule/.test( code );
 
 	walk( ast, {
 		enter ( node, parent ) {
+			if ( sourceMap ) {
+				magicString.addSourcemapLocation( node.start );
+				magicString.addSourcemapLocation( node.end );
+			}
+
 			// skip dead branches
 			if ( parent && ( parent.type === 'IfStatement' || parent.type === 'ConditionalExpression' ) ) {
 				if ( node === parent.consequent && isFalsy( parent.test ) ) return this.skip();
 				if ( node === parent.alternate && isTruthy( parent.test ) ) return this.skip();
 			}
 
-			if ( node.scope ) scope = node.scope;
-			if ( /^Function/.test( node.type ) ) scopeDepth += 1;
+			if ( node._skip ) return this.skip();
 
-			if ( sourceMap ) {
-				magicString.addSourcemapLocation( node.start );
-				magicString.addSourcemapLocation( node.end );
-			}
+			programDepth += 1;
+
+			if ( node.scope ) scope = node.scope;
+			if ( /^Function/.test( node.type ) ) lexicalDepth += 1;
 
 			// Is this an assignment to exports or module.exports?
 			if ( node.type === 'AssignmentExpression' ) {
@@ -94,6 +104,14 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 				const match = exportsPattern.exec( flattened.keypath );
 				if ( !match || flattened.keypath === 'exports' ) return;
 
+				uses[ flattened.name ] = true;
+
+				// we're dealing with `module.exports = ...` or `[module.]exports.foo = ...` –
+				// if this isn't top-level, we'll need to wrap the module
+				if ( programDepth > 3 ) shouldWrap = true;
+
+				node.left._skip = true;
+
 				if ( flattened.keypath === 'module.exports' && node.right.type === 'ObjectExpression' ) {
 					return node.right.properties.forEach( prop => {
 						if ( prop.computed || prop.key.type !== 'Identifier' ) return;
@@ -103,7 +121,6 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 				}
 
 				if ( match[1] ) namedExports[ match[1] ] = true;
-
 				return;
 			}
 
@@ -120,12 +137,20 @@ 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` );
+					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;
+					}
 				}
 				return;
 			}
 
-			if ( node.type === 'ThisExpression' && scopeDepth === 0 && !ignoreGlobal ) {
+			if ( node.type === 'ThisExpression' && lexicalDepth === 0 ) {
 				uses.global = true;
 				if ( !ignoreGlobal ) magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
 				return;
@@ -160,8 +185,9 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 		},
 
 		leave ( node ) {
+			programDepth -= 1;
 			if ( node.scope ) scope = scope.parent;
-			if ( /^Function/.test( node.type ) ) scopeDepth -= 1;
+			if ( /^Function/.test( node.type ) ) lexicalDepth -= 1;
 		}
 	});
 
@@ -172,7 +198,8 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 		return null; // not a CommonJS module
 	}
 
-	const importBlock = [ `import * as ${HELPERS_NAME} from '${HELPERS_ID}';` ].concat(
+	const includeHelpers = shouldWrap || uses.global;
+	const importBlock = ( includeHelpers ? [ `import * as ${HELPERS_NAME} from '${HELPERS_ID}';` ] : [] ).concat(
 		sources.map( source => {
 			// import the actual module before the proxy, so that we know
 			// what kind of proxy to build
@@ -182,27 +209,86 @@ export default function transform ( code, id, isEntry, ignoreGlobal, customNamed
 			const { name, importsDefault } = required[ source ];
 			return `import ${importsDefault ? `${name} from ` : ``}'${PREFIX}${source}';`;
 		})
-	).join( '\n' );
+	).join( '\n' ) + '\n\n';
 
-	const args = `module${uses.exports ? ', exports' : ''}`;
+	const namedExportDeclarations = [];
+	let wrapperStart = '';
+	let wrapperEnd = '';
 
-	const name = getName( id );
+	const moduleName = deconflict( scope, getName( id ) );
+	if ( !isEntry ) {
+		const exportModuleExports = `export { ${moduleName} as __moduleExports };`;
+		namedExportDeclarations.push( exportModuleExports );
+	}
+
+	if ( shouldWrap ) {
+		const args = `module${uses.exports ? ', exports' : ''}`;
+
+		const name = getName( id );
 
-	const wrapperStart = `\n\nvar ${name} = ${HELPERS_NAME}.createCommonjsModule(function (${args}) {\n`;
-	const wrapperEnd = `\n});\n\n`;
+		wrapperStart = `var ${moduleName} = ${HELPERS_NAME}.createCommonjsModule(function (${args}) {\n`;
+		wrapperEnd = `\n});`;
 
-	const exportBlock = ( isEntry ? [] : [ `export { ${name} as __moduleExports };` ] ).concat(
-		/__esModule/.test( code ) ? `export default ${HELPERS_NAME}.unwrapExports(${name});\n` : `export default ${name};\n`,
 		Object.keys( namedExports )
 			.filter( key => !blacklistedExports[ key ] )
-			.map( x => {
-				if (x === name) {
-					return `var ${x}$$1 = ${name}.${x};\nexport { ${x}$$1 as ${x} };`;
+			.forEach( x => {
+				let declaration;
+
+				if ( x === name ) {
+					const deconflicted = deconflict( scope, name );
+					declaration = `var ${deconflicted} = ${moduleName}.${x};\nexport { ${deconflicted} as ${x} };`;
+				} else {
+					declaration = `export var ${x} = ${moduleName}.${x};`;
+				}
+
+				namedExportDeclarations.push( declaration );
+			});
+	} else {
+		let hasDefaultExport = false;
+		const names = [];
+
+		ast.body.forEach( node => {
+			if ( node.type === 'ExpressionStatement' && node.expression.type === 'AssignmentExpression' ) {
+				const { left, right } = node.expression;
+				const flattened = flatten( left );
+
+				if ( !flattened ) return;
+
+				const match = exportsPattern.exec( flattened.keypath );
+				if ( !match ) return;
+
+				if ( flattened.keypath === 'module.exports' ) {
+					hasDefaultExport = true;
+					magicString.overwrite( node.start, right.start, `var ${moduleName} = ` );
 				} else {
-					return `export var ${x} = ${name}.${x};`;
+					const name = match[1];
+					const deconflicted = deconflict( scope, name );
+
+					names.push({ name, deconflicted });
+
+					magicString.overwrite( node.start, right.start, `var ${deconflicted} = ` );
+
+					const declaration = name === deconflicted ?
+						`export { ${name} };` :
+						`export { ${deconflicted} as ${name} };`;
+
+					namedExportDeclarations.push( declaration );
 				}
-			})
-	).join( '\n' );
+			}
+		});
+
+		if ( !hasDefaultExport ) {
+			wrapperEnd = `\n\nvar ${moduleName} = {\n${
+				names.map( ({ name, deconflicted }) => `\t${name}: ${deconflicted}` ).join( ',\n' )
+			}\n};`;
+		}
+	}
+
+	const defaultExport = /__esModule/.test( code ) ?
+		`export default ${HELPERS_NAME}.unwrapExports(${moduleName});` :
+		`export default ${moduleName};`;
+
+	const exportBlock = '\n\n' + [ defaultExport ].concat( namedExportDeclarations ).join( '\n' );
 
 	magicString.trim()
 		.prepend( importBlock + wrapperStart )

test/form/optimised-default-export-function/input.js

@@ -0,0 +1 @@
+module.exports = function foo () {};

test/form/optimised-default-export-function/output.js

@@ -0,0 +1,4 @@
+var input = function foo () {};
+
+export default input;
+export { input as __moduleExports };

test/form/optimised-default-export/input.js

@@ -0,0 +1 @@
+module.exports = 42;

test/form/optimised-default-export/output.js

@@ -0,0 +1,4 @@
+var input = 42;
+
+export default input;
+export { input as __moduleExports };

test/form/optimised-named-export-conflicts/input.js

@@ -0,0 +1,5 @@
+var foo = 1;
+var bar = 2;
+
+exports.foo = 'a';
+module.exports.bar = 'b';

test/form/optimised-named-export-conflicts/output.js

@@ -0,0 +1,15 @@
+var foo = 1;
+var bar = 2;
+
+var foo_1 = 'a';
+var bar_1 = 'b';
+
+var input = {
+	foo: foo_1,
+	bar: bar_1
+};
+
+export default input;
+export { input as __moduleExports };
+export { foo_1 as foo };
+export { bar_1 as bar };

test/form/optimised-named-export/input.js

@@ -0,0 +1,2 @@
+exports.foo = 'a';
+module.exports.bar = 'b';

test/form/optimised-named-export/output.js

@@ -0,0 +1,12 @@
+var foo = 'a';
+var bar = 'b';
+
+var input = {
+	foo: foo,
+	bar: bar
+};
+
+export default input;
+export { input as __moduleExports };
+export { foo };
+export { bar };

test/test.js

@@ -11,11 +11,7 @@ require( 'source-map-support' ).install();
 
 process.chdir( __dirname );
 
-function executeBundle ( bundle ) {
-	const { code } = bundle.generate({
-		format: 'cjs'
-	});
-
+function execute ( code ) {
 	let fn;
 
 	try {
@@ -38,6 +34,11 @@ function executeBundle ( bundle ) {
 	};
 }
 
+function executeBundle ( bundle ) {
+	const { code } = bundle.generate({ format: 'cjs' });
+	return execute( code );
+}
+
 describe( 'rollup-plugin-commonjs', () => {
 	describe( 'form', () => {
 		const { transform, options } = commonjs();
@@ -57,7 +58,7 @@ describe( 'rollup-plugin-commonjs', () => {
 				const expected = fs.readFileSync( `form/${dir}/output.js`, 'utf-8' );
 
 				return transform( input, 'input.js' ).then( transformed => {
-					assert.equal( transformed ? transformed.code : input, expected );
+					assert.equal( ( transformed ? transformed.code : input ).trim(), expected.trim() );
 				});
 			});
 		});
@@ -78,9 +79,12 @@ describe( 'rollup-plugin-commonjs', () => {
 					entry: `function/${dir}/main.js`,
 					plugins: [ commonjs() ]
 				}).then( bundle => {
-					const { code, exports, global } = executeBundle( bundle );
+					const { code } = bundle.generate({ format: 'cjs' });
+					if ( config.show || config.solo ) {
+						console.error( code );
+					}
 
-					if ( config.show ) console.error( code );
+					const { exports, global } = execute( code );
 
 					if ( config.exports ) config.exports( exports );
 					if ( config.global ) config.global( global );