Use new context functions, fix issue when resolveId returns an object (…

…#387)

* Update dependencies

* Transform to new plugin hooks

* Use Sets, remove a micro task

* Make transform hook sync

* Switch to using a Map again

* Refine CJS Promise handling

* Switch from buble to babel, add Node 6 and 12 tests

* Get rid of Node12 for now until we figured out how to rewrite tests with globals

* Use external "is-reference"

* Change format of proxy ids to `\0file/path?commonjs-proxy`

* Update dependencies

* fix tests

* Remove Node 6 again from tests

.travis.yml

@@ -6,4 +6,6 @@ node_js:
 env:
   global:
     - BUILD_TIMEOUT=10000
-install: npm install
+install: npm ci --ignore-scripts
+before_install:
+  - if [[ $TRAVIS_NODE_VERSION -lt 8 ]]; then npm install --global npm@5; fi

package.json

@@ -22,31 +22,36 @@
     "README.md"
   ],
   "peerDependencies": {
-    "rollup": ">=0.56.0"
+    "rollup": ">=1.12.0"
   },
   "dependencies": {
     "estree-walker": "^0.6.0",
+    "is-reference": "^1.1.2",
     "magic-string": "^0.25.2",
-    "resolve": "^1.10.0",
-    "rollup-pluginutils": "^2.6.0"
+    "resolve": "^1.10.1",
+    "rollup-pluginutils": "^2.7.0"
   },
   "devDependencies": {
+    "@babel/core": "7.4.4",
+    "@babel/preset-env": "^7.4.4",
+    "@babel/register": "^7.4.4",
     "acorn": "^6.1.1",
     "eslint": "^5.16.0",
-    "eslint-plugin-import": "^2.16.0",
-    "husky": "^1.3.1",
-    "lint-staged": "^8.1.5",
+    "eslint-plugin-import": "^2.17.2",
+    "husky": "^2.3.0",
+    "lint-staged": "^8.1.7",
     "locate-character": "^2.0.5",
-    "mocha": "^6.0.2",
-    "prettier": "^1.16.4",
+    "mocha": "^6.1.4",
+    "prettier": "^1.17.1",
     "require-relative": "^0.8.7",
-    "rollup": "^1.8.0",
-    "rollup-plugin-buble": "^0.19.6",
-    "rollup-plugin-node-resolve": "^4.0.1",
+    "rollup": "^1.12.0",
+    "rollup-plugin-babel": "^4.3.2",
+    "rollup-plugin-json": "^4.0.0",
+    "rollup-plugin-node-resolve": "^4.2.4",
     "shx": "^0.3.2",
     "source-map": "^0.7.3",
-    "source-map-support": "^0.5.11",
-    "typescript": "^3.4.1"
+    "source-map-support": "^0.5.12",
+    "typescript": "^3.4.5"
   },
   "repository": "rollup/rollup-plugin-commonjs",
   "author": "Rich Harris",

rollup.config.js

@@ -1,11 +1,17 @@
-import buble from 'rollup-plugin-buble';
+import babel from 'rollup-plugin-babel';
+import json from 'rollup-plugin-json';
 import pkg from './package.json';
 
 export default {
 	input: 'src/index.js',
 	plugins: [
-		buble({
-			transforms: { dangerousForOf: true }
+		json(),
+		babel({
+			presets: [['@babel/preset-env', {
+				targets: {
+					node: 6
+				}
+			}]]
 		})
 	],
 	external: Object.keys( pkg.dependencies ).concat([ 'fs', 'path' ]),

src/ast-utils.js

@@ -1,17 +1,4 @@
-export 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;
-}
+export {default as isReference} from 'is-reference';
 
 export function flatten(node) {
 	const parts = [];

src/helpers.js

@@ -1,5 +1,11 @@
-export const PROXY_PREFIX = '\0commonjs-proxy-';
-export const EXTERNAL_PREFIX = '\0commonjs-external-';
+export const PROXY_SUFFIX = '?commonjs-proxy';
+export const getProxyId = id => `\0${id}${PROXY_SUFFIX}`;
+export const getIdFromProxyId = proxyId => proxyId.slice(1, -PROXY_SUFFIX.length);
+
+export const EXTERNAL_SUFFIX = '?commonjs-external';
+export const getExternalProxyId = id => `\0${id}${EXTERNAL_SUFFIX}`;
+export const getIdFromExternalProxyId = proxyId => proxyId.slice(1, -EXTERNAL_SUFFIX.length);
+
 export const HELPERS_ID = '\0commonjsHelpers.js';
 
 // `x['default']` is used instead of `x.default` for backward compatibility with ES3 browsers.

src/index.js

@@ -1,7 +1,8 @@
 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 { peerDependencies } from '../package.json';
+import { EXTERNAL_SUFFIX, getIdFromExternalProxyId, getIdFromProxyId, HELPERS, HELPERS_ID, PROXY_SUFFIX } from './helpers';
 import { getIsCjsPromise, setIsCjsPromise } from './is-cjs';
 import { getResolveId } from './resolve-id';
 import { checkEsModule, hasCjsKeywords, transformCommonjs } from './transform.js';
@@ -27,8 +28,8 @@ export default function commonjs(options = {}) {
 		});
 	}
 
-	const esModulesWithoutDefaultExport = Object.create(null);
-	const esModulesWithDefaultExport = Object.create(null);
+	const esModulesWithoutDefaultExport = new Set();
+	const esModulesWithDefaultExport = new Set();
 	const allowDynamicRequire = !!options.ignore; // TODO maybe this should be configurable?
 
 	const ignoreRequire =
@@ -38,24 +39,59 @@ export default function commonjs(options = {}) {
 				? id => options.ignore.includes(id)
 				: () => false;
 
-	let entryModuleIdsPromise = null;
-
 	const resolveId = getResolveId(extensions);
 
 	const sourceMap = options.sourceMap !== false;
 
+	function transformAndCheckExports(code, id) {
+		{
+			const { isEsModule, hasDefaultExport, ast } = checkEsModule(this.parse, code, id);
+			if (isEsModule) {
+				(hasDefaultExport ? esModulesWithDefaultExport : esModulesWithoutDefaultExport).add(id);
+				return null;
+			}
+
+			// it is not an ES module but it does not have CJS-specific elements.
+			if (!hasCjsKeywords(code, ignoreGlobal)) {
+				esModulesWithoutDefaultExport.add(id);
+				return null;
+			}
+
+			const transformed = transformCommonjs(
+				this.parse,
+				code,
+				id,
+				this.getModuleInfo(id).isEntry,
+				ignoreGlobal,
+				ignoreRequire,
+				customNamedExports[id],
+				sourceMap,
+				allowDynamicRequire,
+				ast
+			);
+			if (!transformed) {
+				esModulesWithoutDefaultExport.add(id);
+				return null;
+			}
+
+			return transformed;
+		}
+	}
+
 	return {
 		name: 'commonjs',
 
-		options(options) {
-			resolveId.setRollupOptions(options);
-			const input = options.input || options.entry;
-			const entryModules = Array.isArray(input)
-				? input
-				: typeof input === 'object' && input !== null
-					? Object.values(input)
-					: [input];
-			entryModuleIdsPromise = Promise.all(entryModules.map(entry => resolveId(entry)));
+		buildStart() {
+			const [major, minor] = this.meta.rollupVersion.split('.').map(Number);
+			const minVersion = peerDependencies.rollup.slice(2);
+			const [minMajor, minMinor] = minVersion.split('.').map(Number);
+			if (major < minMajor || (major === minMajor && minor < minMinor)) {
+				this.error(
+					`Insufficient Rollup version: "rollup-plugin-commonjs" requires at least rollup@${minVersion} but found rollup@${
+						this.meta.rollupVersion
+					}.`
+				);
+			}
 		},
 
 		resolveId,
@@ -64,25 +100,25 @@ export default function commonjs(options = {}) {
 			if (id === HELPERS_ID) return HELPERS;
 
 			// generate proxy modules
-			if (id.startsWith(EXTERNAL_PREFIX)) {
-				const actualId = id.slice(EXTERNAL_PREFIX.length);
+			if (id.endsWith(EXTERNAL_SUFFIX)) {
+				const actualId = getIdFromExternalProxyId(id);
 				const name = getName(actualId);
 
 				return `import ${name} from ${JSON.stringify(actualId)}; export default ${name};`;
 			}
 
-			if (id.startsWith(PROXY_PREFIX)) {
-				const actualId = id.slice(PROXY_PREFIX.length);
+			if (id.endsWith(PROXY_SUFFIX)) {
+				const actualId = getIdFromProxyId(id);
 				const name = getName(actualId);
 
 				return getIsCjsPromise(actualId).then(isCjs => {
 					if (isCjs)
 						return `import { __moduleExports } from ${JSON.stringify(
 							actualId
 						)}; export default __moduleExports;`;
-					else if (esModulesWithoutDefaultExport[actualId])
+					else if (esModulesWithoutDefaultExport.has(actualId))
 						return `import * as ${name} from ${JSON.stringify(actualId)}; export default ${name};`;
-					else if (esModulesWithDefaultExport[actualId]) {
+					else if (esModulesWithDefaultExport.has(actualId)) {
 						return `export {default} from ${JSON.stringify(actualId)};`;
 					} else
 						return `import * as ${name} from ${JSON.stringify(
@@ -94,51 +130,20 @@ export default function commonjs(options = {}) {
 
 		transform(code, id) {
 			if (!filter(id) || extensions.indexOf(extname(id)) === -1) {
-				setIsCjsPromise(id, Promise.resolve(null));
+				setIsCjsPromise(id, null);
 				return null;
 			}
 
-			const transformPromise = entryModuleIdsPromise
-				.then(entryModuleIds => {
-					const { isEsModule, hasDefaultExport, ast } = checkEsModule(this.parse, code, id);
-					if (isEsModule) {
-						(hasDefaultExport ? esModulesWithDefaultExport : esModulesWithoutDefaultExport)[
-							id
-						] = true;
-						return null;
-					}
-
-					// it is not an ES module but it does not have CJS-specific elements.
-					if (!hasCjsKeywords(code, ignoreGlobal)) {
-						esModulesWithoutDefaultExport[id] = true;
-						return null;
-					}
-
-					const transformed = transformCommonjs(
-						this.parse,
-						code,
-						id,
-						entryModuleIds.indexOf(id) !== -1,
-						ignoreGlobal,
-						ignoreRequire,
-						customNamedExports[id],
-						sourceMap,
-						allowDynamicRequire,
-						ast
-					);
-					if (!transformed) {
-						esModulesWithoutDefaultExport[id] = true;
-						return null;
-					}
-
-					return transformed;
-				})
-				.catch(err => {
-					this.error(err, err.loc);
-				});
+			let transformed;
+			try {
+				transformed = transformAndCheckExports.call(this, code, id);
+			} catch (err) {
+				transformed = null;
+				this.error(err, err.loc);
+			}
 
-			setIsCjsPromise(id, transformPromise.then(Boolean, () => false));
-			return transformPromise;
+			setIsCjsPromise(id, Boolean(transformed));
+			return transformed;
 		}
 	};
 }

src/is-cjs.js

@@ -1,28 +1,29 @@
-const isCjsPromises = Object.create(null);
+const isCjsPromises = new Map();
 
 export function getIsCjsPromise(id) {
-	let isCjsPromise = isCjsPromises[id];
+	let isCjsPromise = isCjsPromises.get(id);
 	if (isCjsPromise) return isCjsPromise.promise;
 
 	const promise = new Promise(resolve => {
-		isCjsPromises[id] = isCjsPromise = {
+		isCjsPromise = {
 			resolve,
 			promise: undefined
 		};
+		isCjsPromises.set(id, isCjsPromise);
 	});
 	isCjsPromise.promise = promise;
 
 	return promise;
 }
 
-export function setIsCjsPromise(id, promise) {
-	const isCjsPromise = isCjsPromises[id];
+export function setIsCjsPromise(id, resolution) {
+	const isCjsPromise = isCjsPromises.get(id);
 	if (isCjsPromise) {
 		if (isCjsPromise.resolve) {
-			isCjsPromise.resolve(promise);
+			isCjsPromise.resolve(resolution);
 			isCjsPromise.resolve = undefined;
 		}
 	} else {
-		isCjsPromises[id] = { promise, resolve: undefined };
+		isCjsPromises.set(id, { promise: Promise.resolve(resolution), resolve: undefined });
 	}
 }