rollup-plugin-commonjs - npm Package Compare versions

Comparing version 1.1.0 to 1.2.0

CHANGELOG.md

 # rollup-plugin-commonjs changelog
 
+## 1.2.0
+
+* Generate named exports where possible ([#5](https://github.com/rollup/rollup-plugin-commonjs/issues/5))
+* Handle shadowed `require`/`module`/`exports`
+
 ## 1.1.0
@@ -4,0 +9,0 @@

dist/rollup-plugin-commonjs.cjs.js

@@ -12,3 +12,45 @@ 'use strict';
 
+function isReference(node, parent) {
+	if (parent.type === 'MemberExpression') return parent.computed || node === parent.object;
+
+	// disregard the `bar` in { bar: foo }
+	if (parent.type === 'Property' && node !== parent.value) return false;
+
+	// disregard the `bar` in `class Foo { bar () {...} }`
+	if (parent.type === 'MethodDefinition') return false;
+
+	// disregard the `bar` in `export { foo as bar }`
+	if (parent.type === 'ExportSpecifier' && node !== parent.local) return false;
+
+	return true;
+}
+
+function flatten(node) {
+	var name = undefined;
+	var parts = [];
+
+	while (node.type === 'MemberExpression') {
+		if (node.computed) return null;
+
+		parts.unshift(node.property.name);
+		node = node.object;
+	}
+
+	if (node.type !== 'Identifier') return null;
+
+	name = node.name;
+	parts.unshift(name);
+
+	return { name: name, keypath: parts.join('.') };
+}
+
 var firstpass = /\b(?:require|module|exports)\b/;
+var exportsPattern = /^(?:module\.)?exports(?:\.([a-zA-Z_$][a-zA-Z_$0-9]*))?$/;
+
+function getName(id) {
+	var base = path.basename(id);
+	var ext = path.extname(base);
+
+	return rollupPluginutils.makeLegalIdentifier(ext.length ? base.slice(0, -ext.length) : base);
+}
 function commonjs() {
@@ -30,3 +72,3 @@ var options = arguments.length <= 0 || arguments[0] === undefined ? {} : arguments[0];
 					if (stats.isFile()) return candidates[i];
-				} catch (err) {}
+				} catch (err) {/* noop */}
 			}
@@ -56,9 +98,11 @@ },
 			var uid = 0;
-			var hasCommonJsExports = false;
 
-			// TODO handle shadowed `require` calls
-			var depth = 0;
+			var scope = rollupPluginutils.attachScopes(ast, 'scope');
+			var namedExports = {};
+			var usesModuleOrExports = undefined;
 
 			estreeWalker.walk(ast, {
 				enter: function enter(node, parent) {
+					if (node.scope) scope = node.scope;
+
 					if (options.sourceMap) {
@@ -69,14 +113,21 @@ magicString.addSourcemapLocation(node.start);
 
-					if (/Function/.test(node.type)) {
-						depth += 1;
-						return;
-					}
+					// Is this an assignment to exports or module.exports?
+					if (node.type === 'AssignmentExpression') {
+						if (node.left.type !== 'MemberExpression') return;
 
-					if (/^(Import|Export)/.test(node.type)) {
+						var flattened = flatten(node.left);
+						if (!flattened) return;
+
+						if (scope.contains(flattened.name)) return;
+
+						var match = exportsPattern.exec(flattened.keypath);
+						if (!match || flattened.keypath === 'exports') return;
+
+						if (match[1]) namedExports[match[1]] = true;
+
 						return;
 					}
 
-					// TODO more accurate check
-					if (node.type === 'Identifier' && node.name === 'exports' || node.name === 'module') {
-						hasCommonJsExports = true;
+					if (node.type === 'Identifier') {
+						if ((node.name === 'module' || node.name === 'exports') && isReference(node, parent)) usesModuleOrExports = true;
 						return;
@@ -86,3 +137,3 @@ }
 					if (node.type !== 'CallExpression') return;
-					if (node.callee.name !== 'require') return;
+					if (node.callee.name !== 'require' || scope.contains('require')) return;
 					if (node.arguments.length !== 1 || node.arguments[0].type !== 'Literal') return; // TODO handle these weird cases?
@@ -106,3 +157,3 @@
 				leave: function leave(node) {
-					if (/Function/.test(node.type)) depth -= 1;
+					if (node.scope) scope = scope.parent;
 				}
@@ -113,4 +164,6 @@ });
 
-			if (!sources.length && !hasCommonJsExports) return null;
+			if (!sources.length && !usesModuleOrExports) return null; // not a CommonJS module
 
+			var name = getName(id);
+
 			var importBlock = sources.length ? sources.map(function (source) {
@@ -120,5 +173,9 @@ return 'import ' + required[source].name + ' from \'' + source + '\';';
 
-			var intro = '\n\nexport default (function (module) {\nvar exports = module.exports;\n';
-			var outro = '\nreturn module.exports;\n})({exports:{}});';
+			var intro = '\n\nvar ' + name + ' = (function (module) {\nvar exports = module.exports;\n';
+			var outro = '\nreturn module.exports;\n})({exports:{}});\n\nexport default ' + name + ';\n';
 
+			outro += Object.keys(namedExports).map(function (x) {
+				return 'export var ' + x + ' = ' + name + '.' + x + ';';
+			}).join('\n');
+
 			magicString.trim().prepend(importBlock + intro).trim().append(outro);
@@ -125,0 +182,0 @@

dist/rollup-plugin-commonjs.es6.js

 import { statSync } from 'fs';
-import { extname, sep, dirname, resolve } from 'path';
+import { extname, basename, sep, dirname, resolve } from 'path';
 import acorn from 'acorn';
 import { walk } from 'estree-walker';
 import MagicString from 'magic-string';
-import { createFilter } from 'rollup-pluginutils';
+import { makeLegalIdentifier, attachScopes, createFilter } from 'rollup-pluginutils';
 
+function isReference(node, parent) {
+	if (parent.type === 'MemberExpression') return parent.computed || node === parent.object;
+
+	// disregard the `bar` in { bar: foo }
+	if (parent.type === 'Property' && node !== parent.value) return false;
+
+	// disregard the `bar` in `class Foo { bar () {...} }`
+	if (parent.type === 'MethodDefinition') return false;
+
+	// disregard the `bar` in `export { foo as bar }`
+	if (parent.type === 'ExportSpecifier' && node !== parent.local) return false;
+
+	return true;
+}
+
+function flatten(node) {
+	var name = undefined;
+	var parts = [];
+
+	while (node.type === 'MemberExpression') {
+		if (node.computed) return null;
+
+		parts.unshift(node.property.name);
+		node = node.object;
+	}
+
+	if (node.type !== 'Identifier') return null;
+
+	name = node.name;
+	parts.unshift(name);
+
+	return { name: name, keypath: parts.join('.') };
+}
+
 var firstpass = /\b(?:require|module|exports)\b/;
+var exportsPattern = /^(?:module\.)?exports(?:\.([a-zA-Z_$][a-zA-Z_$0-9]*))?$/;
+
+function getName(id) {
+	var base = basename(id);
+	var ext = extname(base);
+
+	return makeLegalIdentifier(ext.length ? base.slice(0, -ext.length) : base);
+}
 function commonjs() {
@@ -25,3 +67,3 @@ var options = arguments.length <= 0 || arguments[0] === undefined ? {} : arguments[0];
 					if (stats.isFile()) return candidates[i];
-				} catch (err) {}
+				} catch (err) {/* noop */}
 			}
@@ -51,9 +93,11 @@ },
 			var uid = 0;
-			var hasCommonJsExports = false;
 
-			// TODO handle shadowed `require` calls
-			var depth = 0;
+			var scope = attachScopes(ast, 'scope');
+			var namedExports = {};
+			var usesModuleOrExports = undefined;
 
 			walk(ast, {
 				enter: function enter(node, parent) {
+					if (node.scope) scope = node.scope;
+
 					if (options.sourceMap) {
@@ -64,14 +108,21 @@ magicString.addSourcemapLocation(node.start);
 
-					if (/Function/.test(node.type)) {
-						depth += 1;
-						return;
-					}
+					// Is this an assignment to exports or module.exports?
+					if (node.type === 'AssignmentExpression') {
+						if (node.left.type !== 'MemberExpression') return;
 
-					if (/^(Import|Export)/.test(node.type)) {
+						var flattened = flatten(node.left);
+						if (!flattened) return;
+
+						if (scope.contains(flattened.name)) return;
+
+						var match = exportsPattern.exec(flattened.keypath);
+						if (!match || flattened.keypath === 'exports') return;
+
+						if (match[1]) namedExports[match[1]] = true;
+
 						return;
 					}
 
-					// TODO more accurate check
-					if (node.type === 'Identifier' && node.name === 'exports' || node.name === 'module') {
-						hasCommonJsExports = true;
+					if (node.type === 'Identifier') {
+						if ((node.name === 'module' || node.name === 'exports') && isReference(node, parent)) usesModuleOrExports = true;
 						return;
@@ -81,3 +132,3 @@ }
 					if (node.type !== 'CallExpression') return;
-					if (node.callee.name !== 'require') return;
+					if (node.callee.name !== 'require' || scope.contains('require')) return;
 					if (node.arguments.length !== 1 || node.arguments[0].type !== 'Literal') return; // TODO handle these weird cases?
@@ -101,3 +152,3 @@
 				leave: function leave(node) {
-					if (/Function/.test(node.type)) depth -= 1;
+					if (node.scope) scope = scope.parent;
 				}
@@ -108,4 +159,6 @@ });
 
-			if (!sources.length && !hasCommonJsExports) return null;
+			if (!sources.length && !usesModuleOrExports) return null; // not a CommonJS module
 
+			var name = getName(id);
+
 			var importBlock = sources.length ? sources.map(function (source) {
@@ -115,5 +168,9 @@ return 'import ' + required[source].name + ' from \'' + source + '\';';
 
-			var intro = '\n\nexport default (function (module) {\nvar exports = module.exports;\n';
-			var outro = '\nreturn module.exports;\n})({exports:{}});';
+			var intro = '\n\nvar ' + name + ' = (function (module) {\nvar exports = module.exports;\n';
+			var outro = '\nreturn module.exports;\n})({exports:{}});\n\nexport default ' + name + ';\n';
 
+			outro += Object.keys(namedExports).map(function (x) {
+				return 'export var ' + x + ' = ' + name + '.' + x + ';';
+			}).join('\n');
+
 			magicString.trim().prepend(importBlock + intro).trim().append(outro);
@@ -120,0 +177,0 @@

package.json

 {
   "name": "rollup-plugin-commonjs",
-  "version": "1.1.0",
+  "version": "1.2.0",
   "devDependencies": {
@@ -10,2 +10,3 @@ "eslint": "^1.7.3",
     "mocha": "^2.3.3",
+    "rollup": "^0.20.3",
     "rollup-plugin-babel": "^1.0.0",
@@ -31,4 +32,4 @@ "source-map": "^0.5.3"
     "magic-string": "^0.7.0",
-    "rollup-pluginutils": "^1.1.0"
+    "rollup-pluginutils": "^1.2.0"
   }
 }

README.md

@@ -34,3 +34,6 @@ # rollup-plugin-commonjs
       include: 'node_modules/**',
-      exclude: [ 'node_modules/foo/**', 'node_modules/bar/**' ]
+      exclude: [ 'node_modules/foo/**', 'node_modules/bar/**' ],
+      
+      // add this if you want to generate sourcemaps later
+      sourceMap: true
     })
@@ -37,0 +40,0 @@ ]

src/index.js

 import { statSync } from 'fs';
-import { dirname, extname, resolve, sep } from 'path';
+import { basename, dirname, extname, resolve, sep } from 'path';
 import acorn from 'acorn';
 import { walk } from 'estree-walker';
 import MagicString from 'magic-string';
-import { addExtension, createFilter } from 'rollup-pluginutils';
+import { attachScopes, createFilter, makeLegalIdentifier } from 'rollup-pluginutils';
+import { flatten, isReference } from './ast-utils.js';
 
+
 var firstpass = /\b(?:require|module|exports)\b/;
+var exportsPattern = /^(?:module\.)?exports(?:\.([a-zA-Z_$][a-zA-Z_$0-9]*))?$/;
 
+function getName ( id ) {
+	const base = basename( id );
+	const ext = extname( base );
+
+	return makeLegalIdentifier( ext.length ? base.slice( 0, -ext.length ) : base );
+}
+
 export default function commonjs ( options = {} ) {
@@ -28,3 +38,3 @@ var filter = createFilter( options.include, options.exclude );
 					if ( stats.isFile() ) return candidates[i];
-				} catch ( err ) {}
+				} catch ( err ) { /* noop */ }
 			}
@@ -54,9 +64,11 @@ },
 			let uid = 0;
-			let hasCommonJsExports = false;
 
-			// TODO handle shadowed `require` calls
-			let depth = 0;
+			let scope = attachScopes( ast, 'scope' );
+			let namedExports = {};
+			let usesModuleOrExports;
 
 			walk( ast, {
 				enter ( node, parent ) {
+					if ( node.scope ) scope = node.scope;
+
 					if ( options.sourceMap ) {
@@ -67,14 +79,21 @@ magicString.addSourcemapLocation( node.start );
 
-					if ( /Function/.test( node.type ) ) {
-						depth += 1;
-						return;
-					}
+					// Is this an assignment to exports or module.exports?
+					if ( node.type === 'AssignmentExpression' ) {
+						if ( node.left.type !== 'MemberExpression' ) return;
 
-					if ( /^(Import|Export)/.test( node.type ) ) {
+						const flattened = flatten( node.left );
+						if ( !flattened ) return;
+
+						if ( scope.contains( flattened.name ) ) return;
+
+						const match = exportsPattern.exec( flattened.keypath );
+						if ( !match || flattened.keypath === 'exports' ) return;
+
+						if ( match[1] ) namedExports[ match[1] ] = true;
+
 						return;
 					}
 
-					// TODO more accurate check
-					if ( node.type === 'Identifier' && node.name === 'exports' || node.name === 'module' ) {
-						hasCommonJsExports = true;
+					if ( node.type === 'Identifier' ) {
+						if ( ( node.name === 'module' || node.name === 'exports' ) && isReference( node, parent ) ) usesModuleOrExports = true;
 						return;
@@ -84,3 +103,3 @@ }
 					if ( node.type !== 'CallExpression' ) return;
-					if ( node.callee.name !== 'require' ) return;
+					if ( node.callee.name !== 'require' || scope.contains( 'require' ) ) return;
 					if ( node.arguments.length !== 1 || node.arguments[0].type !== 'Literal' ) return; // TODO handle these weird cases?
@@ -104,3 +123,3 @@
 				leave ( node ) {
-					if ( /Function/.test( node.type ) ) depth -= 1;
+					if ( node.scope ) scope = scope.parent;
 				}
@@ -111,4 +130,6 @@ });
 
-			if ( !sources.length && !hasCommonJsExports ) return null;
+			if ( !sources.length && !usesModuleOrExports ) return null; // not a CommonJS module
 
+			const name = getName( id );
+
 			const importBlock = sources.length ?
@@ -118,5 +139,7 @@ sources.map( source => `import ${required[ source ].name} from '${source}';` ).join( '\n' ) :
 
-			const intro = `\n\nexport default (function (module) {\nvar exports = module.exports;\n`;
-			const outro = `\nreturn module.exports;\n})({exports:{}});`;
+			const intro = `\n\nvar ${name} = (function (module) {\nvar exports = module.exports;\n`;
+			let outro = `\nreturn module.exports;\n})({exports:{}});\n\nexport default ${name};\n`;
 
+			outro += Object.keys( namedExports ).map( x => `export var ${x} = ${name}.${x};` ).join( '\n' );
+
 			magicString.trim()
@@ -123,0 +146,0 @@ .prepend( importBlock + intro )

No alert changes

Improved metrics

Total package byte prevSize

17849

increased by46.83%

Number of package files

7

increased by16.67%

Lines of code

395

increased by38.6%

Number of lines in readme file

47

increased by6.82%

Worsened metrics

Dev dependency count

8

increased by14.29%

Dependency changes

• Updatedrollup-pluginutils@^1.2.0