Merge branch 'release-8.4.0'

CHANGELOG.md

@@ -1,5 +1,11 @@
 # rollup-plugin-commonjs changelog
 
+## 8.4.0
+
+* Better handle non-CJS files that contain CJS keywords ([#285](https://github.com/rollup/rollup-plugin-commonjs/issues/285))
+* Use rollup's plugin context`parse` function ([#287](https://github.com/rollup/rollup-plugin-commonjs/issues/287))
+* Improve error handling ([#288](https://github.com/rollup/rollup-plugin-commonjs/issues/288))
+
 ## 8.3.0
 
 * Handle multiple entry points ([#283](https://github.com/rollup/rollup-plugin-commonjs/issues/283))

package.json

@@ -21,18 +21,18 @@
     "README.md"
   ],
   "dependencies": {
-    "acorn": "^5.2.1",
     "estree-walker": "^0.5.0",
     "magic-string": "^0.22.4",
     "resolve": "^1.4.0",
     "rollup-pluginutils": "^2.0.1"
   },
   "devDependencies": {
+    "acorn": "^5.2.1",
     "eslint": "^4.8.0",
     "locate-character": "^2.0.1",
     "mocha": "^4.0.1",
     "require-relative": "^0.8.7",
-    "rollup": "^0.55.0",
+    "rollup": "^0.56.3",
     "rollup-plugin-buble": "^0.16.0",
     "rollup-plugin-node-resolve": "^3.0.0",
     "shx": "^0.2.2",

src/index.js

@@ -169,7 +169,7 @@ export default function commonjs ( options = {} ) {
 			if ( extensions.indexOf( extname( id ) ) === -1 ) return null;
 
 			return entryModuleIdsPromise.then( (entryModuleIds) => {
-				const {isEsModule, hasDefaultExport, ast} = checkEsModule( code, id );
+				const {isEsModule, hasDefaultExport, ast} = checkEsModule( this.parse, code, id );
 				if ( isEsModule ) {
 					if ( !hasDefaultExport )
 						esModulesWithoutDefaultExport.push( id );
@@ -182,11 +182,16 @@ export default function commonjs ( options = {} ) {
 					return;
 				}
 
-				const transformed = transformCommonjs( code, id, entryModuleIds.indexOf(id) !== -1, ignoreGlobal, ignoreRequire, customNamedExports[ id ], sourceMap, allowDynamicRequire, ast );
-				if ( !transformed ) return;
+				const transformed = transformCommonjs( this.parse, code, id, entryModuleIds.indexOf(id) !== -1, ignoreGlobal, ignoreRequire, customNamedExports[ id ], sourceMap, allowDynamicRequire, ast );
+				if ( !transformed ) {
+					esModulesWithoutDefaultExport.push( id );
+					return;
+				}
 
 				commonjsModules.set( id, true );
 				return transformed;
+			}).catch(err => {
+				this.error(err, err.loc);
 			});
 		}
 	};

src/transform.js

@@ -1,4 +1,3 @@
-import acorn from 'acorn';
 import { walk } from 'estree-walker';
 import MagicString from 'magic-string';
 import { attachScopes, makeLegalIdentifier } from 'rollup-pluginutils';
@@ -26,13 +25,9 @@ function deconflict ( scope, globals, identifier ) {
 	return deconflicted;
 }
 
-function tryParse ( code, id ) {
+function tryParse ( parse, code, id ) {
 	try {
-		return acorn.parse( code, {
-			ecmaVersion: 8,
-			sourceType: 'module',
-			allowReturnOutsideFunction: true
-		});
+		return parse( code, { allowReturnOutsideFunction: true });
 	} catch ( err ) {
 		err.message += ` in ${id}`;
 		throw err;
@@ -44,8 +39,8 @@ export function checkFirstpass (code, ignoreGlobal) {
 	return firstpass.test(code);
 }
 
-export function checkEsModule (code, id) {
-	const ast = tryParse(code, id);
+export function checkEsModule ( parse, code, id ) {
+	const ast = tryParse( parse, code, id );
 
 	// if there are top-level import/export declarations, this is ES not CommonJS
 	let hasDefaultExport = false;
@@ -60,8 +55,8 @@ export function checkEsModule (code, id) {
 	return { isEsModule, hasDefaultExport, ast };
 }
 
-export function transformCommonjs ( code, id, isEntry, ignoreGlobal, ignoreRequire, customNamedExports, sourceMap, allowDynamicRequire, astCache ) {
-	const ast = astCache || tryParse( code, id );
+export function transformCommonjs ( parse, code, id, isEntry, ignoreGlobal, ignoreRequire, customNamedExports, sourceMap, allowDynamicRequire, astCache ) {
+	const ast = astCache || tryParse( parse, code, id );
 
 	const magicString = new MagicString( code );
 

test/function/bare-import-comment/bar.js

@@ -0,0 +1,2 @@
+// Great module
+Math.bar = 42;

test/function/bare-import-comment/foo.js

@@ -0,0 +1 @@
+require( './bar.js' );

test/function/bare-import-comment/main.js

@@ -0,0 +1,3 @@
+import './foo.js';
+
+assert.equal( Math.bar, 42 );

test/samples/invalid-syntax/main.js

@@ -0,0 +1 @@
+export const foo = 2,

test/test.js

@@ -1,3 +1,4 @@
+const acorn = require( 'acorn' );
 const path = require( 'path' );
 const fs = require( 'fs' );
 const assert = require( 'assert' );
@@ -55,6 +56,14 @@ async function executeBundle ( bundle, { context, exports } = {} ) {
 	return execute( code, context );
 }
 
+const transformContext = {
+	parse: ( input, options ) =>
+		acorn.parse( input, Object.assign( {
+			ecmaVersion: 9,
+			sourceType: 'module',
+		}, options ) )
+};
+
 describe( 'rollup-plugin-commonjs', () => {
 	describe( 'form', () => {
 		fs.readdirSync( 'form' ).forEach( dir => {
@@ -73,7 +82,7 @@ describe( 'rollup-plugin-commonjs', () => {
 				const input = fs.readFileSync( `form/${dir}/input.js`, 'utf-8' );
 				const expected = fs.readFileSync( `form/${dir}/output.js`, 'utf-8' ).trim();
 
-				return transform( input, 'input.js' ).then( transformed => {
+				return transform.call( transformContext, input, 'input.js' ).then( transformed => {
 					const actual = ( transformed ? transformed.code : input ).trim().replace( /\0/g, '' );
 					assert.equal( actual, expected );
 				});
@@ -481,6 +490,24 @@ describe( 'rollup-plugin-commonjs', () => {
 				onwarn: (warn) => warns.push( warn )
 			});
 			assert.equal( warns.length, 0 );
+
+			await rollup({
+				input: 'function/bare-import-comment/main.js',
+				plugins: [ commonjs() ],
+				onwarn: (warn) => warns.push( warn )
+			});
+			assert.equal( warns.length, 0 );
+		});
+
+		it( 'creates an error with a code frame when parsing fails', async () => {
+			try {
+				await rollup({
+					input: 'samples/invalid-syntax/main.js',
+					plugins: [ commonjs() ]
+				});
+			} catch (error) {
+				assert.equal( error.frame, '1: export const foo = 2,\n                        ^' );
+			}
 		});
 	});
 });