Merge pull request #243 from otolab/feature/1408

add esModulesWithoutDefaultExport check

src/index.js

@@ -4,7 +4,7 @@ import { sync as nodeResolveSync } from 'resolve';
 import { createFilter } from 'rollup-pluginutils';
 import { EXTERNAL, PREFIX, HELPERS_ID, HELPERS } from './helpers.js';
 import defaultResolver from './defaultResolver.js';
-import transformCommonjs from './transform.js';
+import { checkFirstpass, checkEsModule, transformCommonjs } from './transform.js';
 import { getName } from './utils.js';
 
 function getCandidatesForExtension ( resolved, extension ) {
@@ -58,6 +58,8 @@ export default function commonjs ( options = {} ) {
 		});
 	}
 
+	const esModulesWithoutDefaultExport = [];
+
 	const allowDynamicRequire = !!options.ignore; // TODO maybe this should be configurable?
 
 	const ignoreRequire = typeof options.ignore === 'function' ?
@@ -153,9 +155,12 @@ export default function commonjs ( options = {} ) {
 				const actualId = id.slice( PREFIX.length );
 				const name = getName( actualId );
 
-				return commonjsModules.has( actualId ) ?
-					`import { __moduleExports } from ${JSON.stringify( actualId )}; export default __moduleExports;` :
-					`import * as ${name} from ${JSON.stringify( actualId )}; export default ( ${name} && ${name}['default'] ) || ${name};`;
+				if (commonjsModules.has( actualId ))
+					return `import { __moduleExports } from ${JSON.stringify( actualId )}; export default __moduleExports;`;
+				else if (esModulesWithoutDefaultExport.includes(actualId))
+					return `import * as ${name} from ${JSON.stringify( actualId )}; export default ${name};`;
+				else
+					return `import * as ${name} from ${JSON.stringify( actualId )}; export default ( ${name} && ${name}['default'] ) || ${name};`;
 			}
 		},
 
@@ -164,12 +169,24 @@ export default function commonjs ( options = {} ) {
 			if ( extensions.indexOf( extname( id ) ) === -1 ) return null;
 
 			return entryModuleIdPromise.then( () => {
-				const transformed = transformCommonjs( code, id, id === entryModuleId, ignoreGlobal, ignoreRequire, customNamedExports[ id ], sourceMap, allowDynamicRequire );
+				const {isEsModule, hasDefaultExport, ast} = checkEsModule( code, id );
+				if ( isEsModule ) {
+					if ( !hasDefaultExport )
+						esModulesWithoutDefaultExport.push( id );
+					return;
+				}
 
-				if ( transformed ) {
-					commonjsModules.set( id, true );
-					return transformed;
+				// it is not an ES module but not a commonjs module, too.
+				if ( !checkFirstpass( code, ignoreGlobal ) ) {
+					esModulesWithoutDefaultExport.push( id );
+					return;
 				}
+
+				const transformed = transformCommonjs( code, id, id === entryModuleId, ignoreGlobal, ignoreRequire, customNamedExports[ id ], sourceMap, allowDynamicRequire, ast );
+				if ( !transformed ) return;
+
+				commonjsModules.set( id, true );
+				return transformed;
 			});
 		}
 	};

src/transform.js

@@ -39,17 +39,30 @@ function tryParse ( code, id ) {
 	}
 }
 
-export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ignoreRequire, customNamedExports, sourceMap, allowDynamicRequire ) {
+export function checkFirstpass (code, ignoreGlobal) {
 	const firstpass = ignoreGlobal ? firstpassNoGlobal : firstpassGlobal;
-	if ( !firstpass.test( code ) ) return null;
+	return firstpass.test(code);
+}
 
-	const ast = tryParse( code, id );
+export function checkEsModule (code, id) {
+	const ast = tryParse(code, id);
 
 	// if there are top-level import/export declarations, this is ES not CommonJS
+	let hasDefaultExport = false;
+	let isEsModule = false;
 	for ( const node of ast.body ) {
-		if ( importExportDeclaration.test( node.type ) ) return null;
+		if ( node.type === 'ExportDefaultDeclaration' )
+			hasDefaultExport = true;
+		if ( importExportDeclaration.test( node.type ) )
+			isEsModule = true;
 	}
 
+	return { isEsModule, hasDefaultExport, ast };
+}
+
+export function transformCommonjs ( code, id, isEntry, ignoreGlobal, ignoreRequire, customNamedExports, sourceMap, allowDynamicRequire, astCache ) {
+	const ast = astCache || tryParse( code, id );
+
 	const magicString = new MagicString( code );
 
 	const required = {};

test/samples/es-modules-without-default-export/main.js

@@ -0,0 +1,3 @@
+const { a } = require('./other.js');
+
+assert.equal( a, 1 );

test/samples/es-modules-without-default-export/other.js

@@ -0,0 +1 @@
+export const a = 1;

test/test.js

@@ -432,5 +432,22 @@ describe( 'rollup-plugin-commonjs', () => {
 			await executeBundle( bundle, { context: { window } } );
 			assert.notEqual( window.b.default, undefined );
 		});
+
+		it( 'does not warn even if the ES module not export "default"', async () => {
+			const warns = [];
+			await rollup({
+				input: 'samples/es-modules-without-default-export/main.js',
+				plugins: [ commonjs() ],
+				onwarn: (warn) => warns.push( warn )
+			});
+			assert.equal( warns.length, 0 );
+
+			await rollup({
+				input: 'function/bare-import/bar.js',
+				plugins: [ commonjs() ],
+				onwarn: (warn) => warns.push( warn )
+			});
+			assert.equal( warns.length, 0 );
+		});
 	});
-});
+});