Merge pull request #1540 from rollup/gh-1511

Treat exports and re-exports differently
rollup · Aug 11, 2017 · e70dad3 · e70dad3
2 parents 8e7cbe0 + a44f442
commit e70dad3
Show file tree

Hide file tree

Showing 16 changed files with 88 additions and 30 deletions.
diff --git a/src/Bundle.js b/src/Bundle.js
@@ -169,7 +169,18 @@ export default class Bundle {
 					if ( declaration.isNamespace ) {
 						declaration.needsNamespaceBlock = true;
 					}
-				} );
+				});
+
+				entryModule.getReexports().forEach( name => {
+					const declaration = entryModule.traceExport( name );
+
+					if ( declaration.isExternal ) {
+						declaration.reexported = declaration.module.reexported = true;
+					} else {
+						declaration.exportName = name;
+						declaration.activate();
+					}
+				});
 
 				// mark statements that should appear in the bundle
 				if ( this.treeshake ) {
@@ -211,7 +222,7 @@ export default class Bundle {
 				this.externalModules.forEach( module => {
 					const unused = Object.keys( module.declarations )
 						.filter( name => name !== '*' )
-						.filter( name => !module.declarations[ name ].activated );
+						.filter( name => !module.declarations[ name ].activated && !module.declarations[ name ].reexported );
 
 					if ( unused.length === 0 ) return;
 
@@ -471,7 +482,7 @@ export default class Bundle {
 				}
 			} );
 
-			if ( !magicString.toString().trim() && this.entryModule.getExports().length === 0 ) {
+			if ( !magicString.toString().trim() && this.entryModule.getExports().length === 0 && this.entryModule.getReexports().length === 0 ) {
 				this.warn( {
 					code: 'EMPTY_BUNDLE',
 					message: 'Generated an empty bundle'

diff --git a/src/Declaration.js b/src/Declaration.js
@@ -48,7 +48,7 @@ export class SyntheticNamespaceDeclaration {
 		this.needsNamespaceBlock = false;
 
 		this.originals = blank();
-		module.getExports().forEach( name => {
+		module.getExports().concat( module.getReexports() ).forEach( name => {
 			this.originals[ name ] = module.traceExport( name );
 		});
 	}

diff --git a/src/Module.js b/src/Module.js
@@ -303,28 +303,28 @@ export default class Module {
 	}
 
 	getExports () {
-		const exports = blank();
+		return keys( this.exports );
+	}
 
-		keys( this.exports ).forEach( name => {
-			exports[ name ] = true;
-		} );
+	getReexports () {
+		const reexports = blank();
 
 		keys( this.reexports ).forEach( name => {
-			exports[ name ] = true;
+			reexports[ name ] = true;
 		} );
 
 		this.exportAllModules.forEach( module => {
 			if ( module.isExternal ) {
-				exports[ `*${module.id}` ] = true;
+				reexports[ `*${module.id}` ] = true;
 				return;
 			}
 
-			module.getExports().forEach( name => {
-				if ( name !== 'default' ) exports[ name ] = true;
+			module.getExports().concat( module.getReexports() ).forEach( name => {
+				if ( name !== 'default' ) reexports[ name ] = true;
 			} );
 		} );
 
-		return keys( exports );
+		return keys( reexports );
 	}
 
 	namespace () {

diff --git a/src/finalisers/cjs.js b/src/finalisers/cjs.js
@@ -31,7 +31,9 @@ export default function cjs ( bundle, magicString, { exportMode, intro, outro },
 				const activated = Object.keys( module.declarations )
 					.filter( name => module.declarations[ name ].activated );
 
-				return activated.length ?
+				const needsVar = activated.length || module.reexported;
+
+				return needsVar ?
 					`${varOrConst} ${module.name} = require('${module.path}');` :
 					`require('${module.path}');`;
 			}

diff --git a/src/finalisers/es.js b/src/finalisers/es.js
@@ -6,10 +6,6 @@ function notDefault ( name ) {
 
 export default function es ( bundle, magicString, { intro, outro } ) {
 	const importBlock = bundle.externalModules
-		.filter( module => {
-			const imported = keys( module.declarations );
-			return imported.length !== 1 || imported[0][0] !== '*';
-		})
 		.map( module => {
 			const specifiers = [];
 			const specifiersList = [specifiers];
@@ -36,7 +32,7 @@ export default function es ( bundle, magicString, { intro, outro } ) {
 				}
 			}
 
-			const namespaceSpecifier = module.declarations['*'] ? `* as ${module.name}` : null; // TODO prevent unnecessary namespace import, e.g form/external-imports
+			const namespaceSpecifier = module.declarations['*'] && module.declarations['*'].activated ? `* as ${module.name}` : null; // TODO prevent unnecessary namespace import, e.g form/external-imports
 			const namedSpecifier = importedNames.length ? `{ ${importedNames.sort().join( ', ' )} }` : null;
 
 			if ( namespaceSpecifier && namedSpecifier ) {
@@ -50,11 +46,16 @@ export default function es ( bundle, magicString, { intro, outro } ) {
 			}
 
 			return specifiersList
-				.map( specifiers =>
-					specifiers.length ?
-						`import ${specifiers.join( ', ' )} from '${module.path}';` :
-						`import '${module.path}';`
-				)
+				.map( specifiers => {
+					if ( specifiers.length ) {
+						return `import ${specifiers.join( ', ' )} from '${module.path}';`;
+					}
+
+					return module.reexported ?
+						null :
+						`import '${module.path}';`;
+				})
+				.filter( Boolean )
 				.join( '\n' );
 		})
 		.join( '\n' );
@@ -66,7 +67,7 @@ export default function es ( bundle, magicString, { intro, outro } ) {
 
 	const exportAllDeclarations = [];
 
-	const specifiers = module.getExports()
+	const specifiers = module.getExports().concat( module.getReexports() )
 		.filter( notDefault )
 		.map( name => {
 			const declaration = module.traceExport( name );

diff --git a/src/finalisers/shared/getExportBlock.js b/src/finalisers/shared/getExportBlock.js
@@ -5,7 +5,7 @@ export default function getExportBlock ( bundle, exportMode, mechanism = 'return
 		return `${mechanism} ${entryModule.traceExport( 'default' ).getName( false )};`;
 	}
 
-	return entryModule.getExports()
+	const exports = entryModule.getExports().concat( entryModule.getReexports() )
 		.map( name => {
 			if ( name[0] === '*' ) {
 				// export all from external
@@ -27,7 +27,9 @@ export default function getExportBlock ( bundle, exportMode, mechanism = 'return
 			if ( lhs === rhs ) return null;
 
 			return `${lhs} = ${rhs};`;
-		})
+		});
+
+	return exports
 		.filter( Boolean )
 		.join( '\n' );
 }
diff --git a/test/form/samples/external-imports/_expected/es.js b/test/form/samples/external-imports/_expected/es.js
@@ -1,7 +1,6 @@
 import factory from 'factory';
 import { bar, foo } from 'baz';
 import { forEach, port } from 'shipping-port';
-import * as containers from 'shipping-port';
 import alphabet, { a } from 'alphabet';
 
 factory( null );

diff --git a/test/form/samples/import-external-namespace-and-default/_expected/es.js b/test/form/samples/import-external-namespace-and-default/_expected/es.js
@@ -1,6 +1,5 @@
 import { bar } from 'foo';
 import foo__default from 'foo';
-import * as foo from 'foo';
 
 console.log( bar );
 

diff --git a/test/form/samples/import-namespace/_config.js b/test/form/samples/import-namespace/_config.js
@@ -0,0 +1,5 @@
+module.exports = {
+	options: {
+		external: ['foo', 'bar']
+	}
+};
diff --git a/test/form/samples/import-namespace/_expected/amd.js b/test/form/samples/import-namespace/_expected/amd.js
@@ -0,0 +1,6 @@
+define(['foo', 'bar'], function (foo, bar) { 'use strict';
+
+	foo.x();
+	console.log(bar);
+
+});
diff --git a/test/form/samples/import-namespace/_expected/cjs.js b/test/form/samples/import-namespace/_expected/cjs.js
@@ -0,0 +1,7 @@
+'use strict';
+
+var foo = require('foo');
+var bar = require('bar');
+
+foo.x();
+console.log(bar);
diff --git a/test/form/samples/import-namespace/_expected/es.js b/test/form/samples/import-namespace/_expected/es.js
@@ -0,0 +1,5 @@
+import { x } from 'foo';
+import * as bar from 'bar';
+
+x();
+console.log(bar);
diff --git a/test/form/samples/import-namespace/_expected/iife.js b/test/form/samples/import-namespace/_expected/iife.js
@@ -0,0 +1,7 @@
+(function (foo,bar) {
+	'use strict';
+
+	foo.x();
+	console.log(bar);
+
+}(foo,bar));
diff --git a/test/form/samples/import-namespace/_expected/umd.js b/test/form/samples/import-namespace/_expected/umd.js
@@ -0,0 +1,10 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('foo'), require('bar')) :
+	typeof define === 'function' && define.amd ? define(['foo', 'bar'], factory) :
+	(factory(global.foo,global.bar));
+}(this, (function (foo,bar) { 'use strict';
+
+	foo.x();
+	console.log(bar);
+
+})));
diff --git a/test/form/samples/import-namespace/main.js b/test/form/samples/import-namespace/main.js
@@ -0,0 +1,5 @@
+import * as foo from 'foo';
+import * as bar from 'bar';
+
+foo.x();
+console.log(bar);
diff --git a/test/form/samples/no-treeshake/_expected/es.js b/test/form/samples/no-treeshake/_expected/es.js
@@ -1,5 +1,4 @@
 import { value } from 'external';
-import * as external from 'external';
 
 var foo = 'unused';