Prepare for 1.0.0, fix some issues (#349)

* Fix warning when importing ESM from CJS and optimize generated code * Make tests compatible with rollup 1.0 * Resolve #348 * Resolve #341
rollup · Oct 10, 2018 · 4a22147 · 4a22147
1 parent 741df7f
commit 4a22147
Show file tree

Hide file tree

Showing 14 changed files with 255 additions and 94 deletions.
diff --git a/src/helpers.js b/src/helpers.js
@@ -10,9 +10,13 @@ export function commonjsRequire () {
 }
 
 export function unwrapExports (x) {
-	return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;
+	return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x.default : x;
 }
 
 export function createCommonjsModule(fn, module) {
 	return module = { exports: {} }, fn(module, module.exports), module.exports;
+}
+
+export function getCjsExportFromNamespace (n) {
+	return n && n.default || n;
 }`;
diff --git a/src/index.js b/src/index.js
@@ -1,11 +1,11 @@
-import {extname, resolve} from 'path';
-import {sync as nodeResolveSync} from 'resolve';
-import {createFilter} from 'rollup-pluginutils';
-import {EXTERNAL_PREFIX, HELPERS, HELPERS_ID, PROXY_PREFIX} from './helpers.js';
-import {getIsCjsPromise, setIsCjsPromise} from './is-cjs';
-import {getResolveId} from './resolve-id';
-import {checkEsModule, hasCjsKeywords, transformCommonjs} from './transform.js';
-import {getName} from './utils.js';
+import { extname, resolve } from 'path';
+import { sync as nodeResolveSync } from 'resolve';
+import { createFilter } from 'rollup-pluginutils';
+import { EXTERNAL_PREFIX, HELPERS, HELPERS_ID, PROXY_PREFIX } from './helpers.js';
+import { getIsCjsPromise, setIsCjsPromise } from './is-cjs';
+import { getResolveId } from './resolve-id';
+import { checkEsModule, hasCjsKeywords, transformCommonjs } from './transform.js';
+import { getName } from './utils.js';
 
 export default function commonjs(options = {}) {
 	const extensions = options.extensions || ['.js'];
@@ -27,7 +27,8 @@ export default function commonjs(options = {}) {
 		});
 	}
 
-	const esModulesWithoutDefaultExport = [];
+	const esModulesWithoutDefaultExport = Object.create(null);
+	const esModulesWithDefaultExport = Object.create(null);
 	const allowDynamicRequire = !!options.ignore; // TODO maybe this should be configurable?
 
 	const ignoreRequire =
@@ -74,40 +75,42 @@ export default function commonjs(options = {}) {
 				const actualId = id.slice(PROXY_PREFIX.length);
 				const name = getName(actualId);
 
-				return (extensions.indexOf(extname(id)) === -1
-					? Promise.resolve(false)
-					: getIsCjsPromise(actualId)
-				).then(isCjs => {
+				return getIsCjsPromise(actualId).then(isCjs => {
 					if (isCjs)
 						return `import { __moduleExports } from ${JSON.stringify(
 							actualId
 						)}; export default __moduleExports;`;
-					else if (esModulesWithoutDefaultExport.indexOf(actualId) !== -1)
+					else if (esModulesWithoutDefaultExport[actualId])
 						return `import * as ${name} from ${JSON.stringify(actualId)}; export default ${name};`;
+					else if (esModulesWithDefaultExport[actualId]) {
+						return `export {default} from ${JSON.stringify(actualId)};`;
+					}
 					else
 						return `import * as ${name} from ${JSON.stringify(
 							actualId
-						)}; export default ( ${name} && ${name}['default'] ) || ${name};`;
+						)}; import {getCjsExportFromNamespace} from "${HELPERS_ID}"; export default getCjsExportFromNamespace(${name})`;
 				});
 			}
 		},
 
 		transform(code, id) {
-			if (!filter(id)) return null;
-			if (extensions.indexOf(extname(id)) === -1) return null;
+			if (!filter(id) || extensions.indexOf(extname(id)) === -1) {
+				setIsCjsPromise(id, Promise.resolve(null));
+				return null;
+			}
 
 			const transformPromise = entryModuleIdsPromise
 				.then(entryModuleIds => {
 					const { isEsModule, hasDefaultExport, ast } = checkEsModule(this.parse, code, id);
 					if (isEsModule) {
-						if (!hasDefaultExport) esModulesWithoutDefaultExport.push(id);
-						return;
+						(hasDefaultExport ? esModulesWithDefaultExport : esModulesWithoutDefaultExport)[id] = true;
+						return null;
 					}
 
-					// it is not an ES module but not a commonjs module, too.
+					// it is not an ES module but it does not have CJS-specific elements.
 					if (!hasCjsKeywords(code, ignoreGlobal)) {
-						esModulesWithoutDefaultExport.push(id);
-						return;
+						esModulesWithoutDefaultExport[id] = true;
+						return null;
 					}
 
 					const transformed = transformCommonjs(
@@ -123,8 +126,8 @@ export default function commonjs(options = {}) {
 						ast
 					);
 					if (!transformed) {
-						esModulesWithoutDefaultExport.push(id);
-						return;
+						esModulesWithoutDefaultExport[id] = true;
+						return null;
 					}
 
 					return transformed;
@@ -133,8 +136,7 @@ export default function commonjs(options = {}) {
 					this.error(err, err.loc);
 				});
 
-			setIsCjsPromise(id, transformPromise.then(Boolean, () => true));
-
+			setIsCjsPromise(id, transformPromise.then(Boolean, () => false));
 			return transformPromise;
 		}
 	};

diff --git a/src/transform.js b/src/transform.js
@@ -1,9 +1,9 @@
-import {walk} from 'estree-walker';
+import { walk } from 'estree-walker';
 import MagicString from 'magic-string';
-import {attachScopes, makeLegalIdentifier} from 'rollup-pluginutils';
-import {extractNames, flatten, isFalsy, isReference, isTruthy} from './ast-utils.js';
-import {HELPERS_ID, PROXY_PREFIX} from './helpers.js';
-import {getName} from './utils.js';
+import { attachScopes, makeLegalIdentifier } from 'rollup-pluginutils';
+import { extractNames, flatten, isFalsy, isReference, isTruthy } from './ast-utils.js';
+import { HELPERS_ID, PROXY_PREFIX } from './helpers.js';
+import { getName } from './utils.js';
 
 const reserved = 'process location abstract arguments boolean break byte case catch char class const continue debugger default delete do double else enum eval export extends false final finally float for from function goto if implements import in instanceof int interface let long native new null package private protected public return short static super switch synchronized this throw throws transient true try typeof var void volatile while with yield'.split(
 	' '
@@ -46,15 +46,21 @@ export function hasCjsKeywords(code, ignoreGlobal) {
 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;
 	let isEsModule = false;
 	for (const node of ast.body) {
-		if (node.type === 'ExportDefaultDeclaration') hasDefaultExport = true;
-		if (importExportDeclaration.test(node.type)) isEsModule = true;
+		if (node.type === 'ExportDefaultDeclaration')
+			return { isEsModule: true, hasDefaultExport: true, ast };
+		if (node.type === 'ExportNamedDeclaration') {
+			isEsModule = true;
+			for (const specifier of node.specifiers) {
+				if (specifier.exported.name === 'default') {
+					return { isEsModule: true, hasDefaultExport: true, ast };
+				}
+			}
+		} else if (importExportDeclaration.test(node.type)) isEsModule = true;
 	}
 
-	return { isEsModule, hasDefaultExport, ast };
+	return { isEsModule, hasDefaultExport: false, ast };
 }
 
 export function transformCommonjs(

diff --git a/test/function/export-default-from/_config.js b/test/function/export-default-from/_config.js
@@ -0,0 +1 @@
+module.exports = {};
diff --git a/test/function/export-default-from/imported.js b/test/function/export-default-from/imported.js
@@ -0,0 +1 @@
+export default 'default export';
diff --git a/test/function/export-default-from/main.js b/test/function/export-default-from/main.js
@@ -0,0 +1 @@
+assert.equal(require('./reexporter'), 'default export');
diff --git a/test/function/export-default-from/reexporter.js b/test/function/export-default-from/reexporter.js
@@ -0,0 +1 @@
+export {default} from './imported';
diff --git a/test/function/resolve-is-cjs-extension/_config.js b/test/function/resolve-is-cjs-extension/_config.js
@@ -0,0 +1,15 @@
+module.exports = {
+	description: 'always resolve cjs detection even if an imported file has an unknown extension',
+	options: {
+		plugins: [
+			{
+				resolveId(importee) {
+					if (importee === 'second') {
+						return `${__dirname}/second.x`;
+					}
+				}
+			}
+		]
+	},
+	pluginOptions: {}
+};
diff --git a/test/function/resolve-is-cjs-extension/main.js b/test/function/resolve-is-cjs-extension/main.js
@@ -0,0 +1 @@
+assert.equal(require('second').result, 'second' );
diff --git a/test/function/resolve-is-cjs-extension/second.x b/test/function/resolve-is-cjs-extension/second.x
@@ -0,0 +1 @@
+export const result = 'second';
diff --git a/test/function/resolve-is-cjs-filtered/_config.js b/test/function/resolve-is-cjs-filtered/_config.js
@@ -0,0 +1,17 @@
+module.exports = {
+	description: 'always resolve cjs detection even if an imported file is filtered',
+	options: {
+		plugins: [
+			{
+				resolveId(importee) {
+					if (importee === 'second') {
+						return `${__dirname}/second.js`;
+					}
+				}
+			}
+		]
+	},
+	pluginOptions: {
+		include: ['function/resolve-is-cjs-filtered/main.js']
+	}
+};
diff --git a/test/function/resolve-is-cjs-filtered/main.js b/test/function/resolve-is-cjs-filtered/main.js
@@ -0,0 +1 @@
+assert.equal(require('second').result, 'second' );
diff --git a/test/function/resolve-is-cjs-filtered/second.js b/test/function/resolve-is-cjs-filtered/second.js
@@ -0,0 +1 @@
+export const result = 'second';