feat(commonjs): support strict require semantics for circular imports

packages/commonjs/README.md

@@ -13,7 +13,7 @@
 
 ## Requirements
 
-This plugin requires an [LTS](https://github.com/nodejs/Release) Node version (v8.0.0+) and Rollup v2.38.3+.
+This plugin requires an [LTS](https://github.com/nodejs/Release) Node version (v12.0.0+) and Rollup v2.68.0+. If you are using [`@rollup/plugin-node-resolve`](https://github.com/rollup/plugins/tree/master/packages/node-resolve), it should be v13.0.6+.
 
 ## Install
 
@@ -42,15 +42,36 @@ export default {
 
 Then call `rollup` either via the [CLI](https://www.rollupjs.org/guide/en/#command-line-reference) or the [API](https://www.rollupjs.org/guide/en/#javascript-api).
 
+When used together with the node-resolve plugin
+
 ## Options
 
+### `strictRequires`
+
+Type: `"auto" | boolean | "debug" | string[]`<br>
+Default: `"auto"`
+
+By default, this plugin will try to hoist `require` statements as imports to the top of each file. While this works well for many code bases and allows for very efficient ESM output, it does not perfectly capture CommonJS semantics as the initialisation order of required modules will be different. The resultant side effects can include log statements being emitted in a different order, and some code that is dependent on the initialisation order of polyfills in require statements may not work. But it is especially problematic when there are circular `require` calls between CommonJS modules as those often rely on the lazy execution of nested `require` calls.
+
+Setting this option to `true` will wrap all CommonJS files in functions which are executed when they are required for the first time, preserving NodeJS semantics. This is the safest setting and should be used if the generated code does not work correctly with `"auto"`. Note that `strictRequires: true` can have a small impact on the size and performance of generated code, but less so if the code is minified.
+
+The default value of `"auto"` will only wrap CommonJS files when they are part of a CommonJS dependency cycle, e.g. an index file that is required by some of its dependencies, or if they are only required in a potentially "conditional" way like from within an if-statement or a function. All other CommonJS files are hoisted. This is the recommended setting for most code bases. Note that the detection of conditional requires can be subject to race conditions if there are both conditional and unconditional requires of the same file, which in edge cases may result in inconsistencies between builds. If you think this is a problem for you, you can avoid this by using any value other than `"auto"` or `"debug"`.
+
+`false` will entirely prevent wrapping and hoist all files. This may still work depending on the nature of cyclic dependencies but will often cause problems.
+
+You can also provide a [minimatch pattern](https://github.com/isaacs/minimatch), or array of patterns, to only specify a subset of files which should be wrapped in functions for proper `require` semantics.
+
+`"debug"` works like `"auto"` but after bundling, it will display a warning containing a list of ids that have been wrapped which can be used as minimatch pattern for fine-tuning or to avoid the potential race conditions mentioned for `"auto"`.
+
 ### `dynamicRequireTargets`
 
 Type: `string | string[]`<br>
 Default: `[]`
 
+_Note: In previous versions, this option would spin up a rather comprehensive mock environment that was capable of handling modules that manipulate `require.cache`. This is no longer supported. If you rely on this e.g. when using request-promise-native, use version 21 of this plugin._
+
 Some modules contain dynamic `require` calls, or require modules that contain circular dependencies, which are not handled well by static imports.
-Including those modules as `dynamicRequireTargets` will simulate a CommonJS (NodeJS-like) environment for them with support for dynamic and circular dependencies.
+Including those modules as `dynamicRequireTargets` will simulate a CommonJS (NodeJS-like) environment for them with support for dynamic dependencies. It also enables `strictRequires` for those modules, see above.
 
 _Note: In extreme cases, this feature may result in some paths being rendered as absolute in the final bundle. The plugin tries to avoid exposing paths from the local machine, but if you are `dynamicRequirePaths` with paths that are far away from your project's folder, that may require replacing strings like `"/Users/John/Desktop/foo-project/"` -> `"/"`._
 
@@ -71,6 +92,13 @@ commonjs({
 });
 ```
 
+### `dynamicRequireRoot`
+
+Type: `string`<br>
+Default: `process.cwd()`
+
+To avoid long paths when using the `dynamicRequireTargets` option, you can use this option to specify a directory that is a common parent for all files that use dynamic require statements. Using a directory higher up such as `/` may lead to unnecessarily long paths in the generated code and may expose directory names on your machine like your home directory name. By default it uses the current working directory.
+
 ### `exclude`
 
 Type: `string | string[]`<br>
@@ -125,15 +153,17 @@ Sometimes you have to leave require statements unconverted. Pass an array contai
 Type: `boolean | 'remove' | string[] | ((id: string) => boolean)`<br>
 Default: `true`
 
-In most cases, where `require` calls are inside a `try-catch` clause, they should be left unconverted as it requires an optional dependency that may or may not be installed beside the rolled up package.
+In most cases, where `require` calls to external dependencies are inside a `try-catch` clause, they should be left unconverted as it requires an optional dependency that may or may not be installed beside the rolled up package.
 Due to the conversion of `require` to a static `import` - the call is hoisted to the top of the file, outside of the `try-catch` clause.
 
-- `true`: All `require` calls inside a `try` will be left unconverted.
-- `false`: All `require` calls inside a `try` will be converted as if the `try-catch` clause is not there.
-- `remove`: Remove all `require` calls from inside any `try` block.
+- `true`: All external `require` calls inside a `try` will be left unconverted.
+- `false`: All external `require` calls inside a `try` will be converted as if the `try-catch` clause is not there.
+- `remove`: Remove all external `require` calls from inside any `try` block.
 - `string[]`: Pass an array containing the IDs to left unconverted.
 - `((id: string) => boolean|'remove')`: Pass a function that control individual IDs.
 
+Note that non-external requires will not be ignored by this option.
+
 ### `ignoreDynamicRequires`
 
 Type: `boolean`
@@ -361,7 +391,7 @@ export default {
     format: 'iife',
     name: 'MyModule'
   },
-  plugins: [resolve(), commonjs()]
+  plugins: [commonjs(), resolve()]
 };
 ```
 

packages/commonjs/package.json

@@ -13,10 +13,14 @@
   "author": "Rich Harris <richard.a.harris@gmail.com>",
   "homepage": "https://github.com/rollup/plugins/tree/master/packages/commonjs/#readme",
   "bugs": "https://github.com/rollup/plugins/issues",
-  "main": "dist/index.js",
-  "module": "dist/index.es.js",
+  "main": "./dist/cjs/index.js",
+  "module": "./dist/es/index.js",
+  "exports": {
+    "require": "./dist/cjs/index.js",
+    "import": "./dist/es/index.js"
+  },
   "engines": {
-    "node": ">= 8.0.0"
+    "node": ">= 12.0.0"
   },
   "scripts": {
     "build": "rollup -c",
@@ -26,6 +30,7 @@
     "ci:test": "pnpm test -- --verbose && pnpm test:ts",
     "prebuild": "del-cli dist",
     "prepare": "if [ ! -d 'dist' ]; then pnpm build; fi",
+    "prepublishOnly": "pnpm build",
     "prerelease": "pnpm build",
     "pretest": "pnpm build",
     "release": "pnpm plugin:release --workspace-root -- --pkg $npm_package_name",
@@ -47,7 +52,7 @@
     "require"
   ],
   "peerDependencies": {
-    "rollup": "^2.38.3"
+    "rollup": "^2.68.0"
   },
   "dependencies": {
     "@rollup/pluginutils": "^3.1.0",
@@ -60,10 +65,10 @@
   },
   "devDependencies": {
     "@rollup/plugin-json": "^4.1.0",
-    "@rollup/plugin-node-resolve": "^8.4.0",
+    "@rollup/plugin-node-resolve": "^13.1.0",
     "locate-character": "^2.0.5",
     "require-relative": "^0.8.7",
-    "rollup": "^2.67.3",
+    "rollup": "^2.68.0",
     "shx": "^0.3.2",
     "source-map": "^0.7.3",
     "source-map-support": "^0.5.19",

packages/commonjs/rollup.config.js

@@ -1,5 +1,7 @@
 import json from '@rollup/plugin-json';
 
+import { emitModulePackageFile } from '../../shared/rollup.config';
+
 import pkg from './package.json';
 
 export default {
@@ -10,6 +12,7 @@ export default {
     {
       file: pkg.module,
       format: 'es',
+      plugins: [emitModulePackageFile()],
       sourcemap: true
     },
     {

packages/commonjs/src/dynamic-modules.js

@@ -0,0 +1,189 @@
+import { existsSync, readFileSync, statSync } from 'fs';
+import { join, resolve, dirname } from 'path';
+
+import getCommonDir from 'commondir';
+
+import glob from 'glob';
+
+import { getVirtualPathForDynamicRequirePath, normalizePathSlashes } from './utils';
+
+function getPackageEntryPoint(dirPath) {
+  let entryPoint = 'index.js';
+
+  try {
+    if (existsSync(join(dirPath, 'package.json'))) {
+      entryPoint =
+        JSON.parse(readFileSync(join(dirPath, 'package.json'), { encoding: 'utf8' })).main ||
+        entryPoint;
+    }
+  } catch (ignored) {
+    // ignored
+  }
+
+  return entryPoint;
+}
+
+function isDirectory(path) {
+  try {
+    if (statSync(path).isDirectory()) return true;
+  } catch (ignored) {
+    // Nothing to do here
+  }
+  return false;
+}
+
+export function getDynamicRequireModules(patterns, dynamicRequireRoot) {
+  const dynamicRequireModules = new Map();
+  const dirNames = new Set();
+  for (const pattern of !patterns || Array.isArray(patterns) ? patterns || [] : [patterns]) {
+    const isNegated = pattern.startsWith('!');
+    const modifyMap = (targetPath, resolvedPath) =>
+      isNegated
+        ? dynamicRequireModules.delete(targetPath)
+        : dynamicRequireModules.set(targetPath, resolvedPath);
+    for (const path of glob.sync(isNegated ? pattern.substr(1) : pattern)) {
+      const resolvedPath = resolve(path);
+      const requirePath = normalizePathSlashes(resolvedPath);
+      if (isDirectory(resolvedPath)) {
+        dirNames.add(resolvedPath);
+        const modulePath = resolve(join(resolvedPath, getPackageEntryPoint(path)));
+        modifyMap(requirePath, modulePath);
+        modifyMap(normalizePathSlashes(modulePath), modulePath);
+      } else {
+        dirNames.add(dirname(resolvedPath));
+        modifyMap(requirePath, resolvedPath);
+      }
+    }
+  }
+  return {
+    commonDir: dirNames.size ? getCommonDir([...dirNames, dynamicRequireRoot]) : null,
+    dynamicRequireModules
+  };
+}
+
+const FAILED_REQUIRE_ERROR = `throw new Error('Could not dynamically require "' + path + '". Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option of @rollup/plugin-commonjs appropriately for this require call to work.');`;
+
+export const COMMONJS_REQUIRE_EXPORT = 'commonjsRequire';
+export const CREATE_COMMONJS_REQUIRE_EXPORT = 'createCommonjsRequire';
+
+export function getDynamicModuleRegistry(
+  isDynamicRequireModulesEnabled,
+  dynamicRequireModules,
+  commonDir,
+  ignoreDynamicRequires
+) {
+  if (!isDynamicRequireModulesEnabled) {
+    return `export function ${COMMONJS_REQUIRE_EXPORT}(path) {
+	${FAILED_REQUIRE_ERROR}
+}`;
+  }
+  const dynamicModuleImports = [...dynamicRequireModules.values()]
+    .map(
+      (id, index) =>
+        `import ${
+          id.endsWith('.json') ? `json${index}` : `{ __require as require${index} }`
+        } from ${JSON.stringify(id)};`
+    )
+    .join('\n');
+  const dynamicModuleProps = [...dynamicRequireModules.keys()]
+    .map(
+      (id, index) =>
+        `\t\t${JSON.stringify(getVirtualPathForDynamicRequirePath(id, commonDir))}: ${
+          id.endsWith('.json') ? `function () { return json${index}; }` : `require${index}`
+        }`
+    )
+    .join(',\n');
+  return `${dynamicModuleImports}
+
+var dynamicModules;
+
+function getDynamicModules() {
+	return dynamicModules || (dynamicModules = {
+${dynamicModuleProps}
+	});
+}
+
+export function ${CREATE_COMMONJS_REQUIRE_EXPORT}(originalModuleDir) {
+	function handleRequire(path) {
+		var resolvedPath = commonjsResolve(path, originalModuleDir);
+		if (resolvedPath !== null) {
+			return getDynamicModules()[resolvedPath]();
+		}
+		${ignoreDynamicRequires ? 'return require(path);' : FAILED_REQUIRE_ERROR}
+	}
+	handleRequire.resolve = function (path) {
+		var resolvedPath = commonjsResolve(path, originalModuleDir);
+		if (resolvedPath !== null) {
+			return resolvedPath;
+		}
+		return require.resolve(path);
+	}
+	return handleRequire;
+}
+
+function commonjsResolve (path, originalModuleDir) {
+	var shouldTryNodeModules = isPossibleNodeModulesPath(path);
+	path = normalize(path);
+	var relPath;
+	if (path[0] === '/') {
+		originalModuleDir = '';
+	}
+	var modules = getDynamicModules();
+	var checkedExtensions = ['', '.js', '.json'];
+	while (true) {
+		if (!shouldTryNodeModules) {
+			relPath = normalize(originalModuleDir + '/' + path);
+		} else {
+			relPath = normalize(originalModuleDir + '/node_modules/' + path);
+		}
+
+		if (relPath.endsWith('/..')) {
+			break; // Travelled too far up, avoid infinite loop
+		}
+
+		for (var extensionIndex = 0; extensionIndex < checkedExtensions.length; extensionIndex++) {
+			var resolvedPath = relPath + checkedExtensions[extensionIndex];
+			if (modules[resolvedPath]) {
+				return resolvedPath;
+			}
+		}
+		if (!shouldTryNodeModules) break;
+		var nextDir = normalize(originalModuleDir + '/..');
+		if (nextDir === originalModuleDir) break;
+		originalModuleDir = nextDir;
+	}
+	return null;
+}
+
+function isPossibleNodeModulesPath (modulePath) {
+	var c0 = modulePath[0];
+	if (c0 === '/' || c0 === '\\\\') return false;
+	var c1 = modulePath[1], c2 = modulePath[2];
+	if ((c0 === '.' && (!c1 || c1 === '/' || c1 === '\\\\')) ||
+		(c0 === '.' && c1 === '.' && (!c2 || c2 === '/' || c2 === '\\\\'))) return false;
+	if (c1 === ':' && (c2 === '/' || c2 === '\\\\')) return false;
+	return true;
+}
+
+function normalize (path) {
+	path = path.replace(/\\\\/g, '/');
+	var parts = path.split('/');
+	var slashed = parts[0] === '';
+	for (var i = 1; i < parts.length; i++) {
+		if (parts[i] === '.' || parts[i] === '') {
+			parts.splice(i--, 1);
+		}
+	}
+	for (var i = 1; i < parts.length; i++) {
+		if (parts[i] !== '..') continue;
+		if (i > 0 && parts[i - 1] !== '..' && parts[i - 1] !== '.') {
+			parts.splice(--i, 2);
+			i--;
+		}
+	}
+	path = parts.join('/');
+	if (slashed && path[0] !== '/') path = '/' + path;
+	else if (path.length === 0) path = '.';
+	return path;
+}`;
+}