feat(commonjs): add dynamicRequireRoot option (#1038)

Author: lukastaegert

packages/commonjs/README.md

@@ -55,13 +55,13 @@ By default, this plugin will try to hoist `require` statements as imports to the
 
 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. Note that this can have an impact on the size and performance of the generated code.
 
-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 many of its dependencies. All other CommonJS files are hoisted. This is the recommended setting for most code bases.
+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.
+`"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`
 
@@ -90,6 +90,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>

packages/commonjs/src/dynamic-modules.js

@@ -1,5 +1,7 @@
 import { existsSync, readFileSync, statSync } from 'fs';
-import { join, resolve } from 'path';
+import { join, resolve, dirname } from 'path';
+
+import getCommonDir from 'commondir';
 
 import glob from 'glob';
 
@@ -30,8 +32,9 @@ function isDirectory(path) {
   return false;
 }
 
-export function getDynamicRequireModules(patterns) {
+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) =>
@@ -42,15 +45,20 @@ export function getDynamicRequireModules(patterns) {
       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 dynamicRequireModules;
+  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.');`;
@@ -77,9 +85,9 @@ export function getDynamicModuleRegistry(
   const dynamicModuleProps = [...dynamicRequireModules.keys()]
     .map(
       (id, index) =>
-        `\t\t${JSON.stringify(
-          getVirtualPathForDynamicRequirePath(normalizePathSlashes(id), commonDir)
-        )}: ${id.endsWith('.json') ? `function () { return json${index}; }` : `require${index}`}`
+        `\t\t${JSON.stringify(getVirtualPathForDynamicRequirePath(id, commonDir))}: ${
+          id.endsWith('.json') ? `function () { return json${index}; }` : `require${index}`
+        }`
     )
     .join(',\n');
   return `${dynamicModuleImports}
@@ -93,7 +101,7 @@ ${dynamicModuleProps}
 }
 
 export function commonjsRequire(path, originalModuleDir) {
-	var resolvedPath = commonjsResolveImpl(path, originalModuleDir, true);
+	var resolvedPath = commonjsResolveImpl(path, originalModuleDir);
 	if (resolvedPath !== null) {
 		return getDynamicModules()[resolvedPath]();
 	}
@@ -115,17 +123,15 @@ function commonjsResolveImpl (path, originalModuleDir) {
 	path = normalize(path);
 	var relPath;
 	if (path[0] === '/') {
-		originalModuleDir = '/';
+		originalModuleDir = '';
 	}
 	var modules = getDynamicModules();
 	var checkedExtensions = ['', '.js', '.json'];
 	while (true) {
 		if (!shouldTryNodeModules) {
-			relPath = originalModuleDir ? normalize(originalModuleDir + '/' + path) : path;
-		} else if (originalModuleDir) {
-			relPath = normalize(originalModuleDir + '/node_modules/' + path);
+			relPath = normalize(originalModuleDir + '/' + path);
 		} else {
-			relPath = normalize(join('node_modules', path));
+			relPath = normalize(originalModuleDir + '/node_modules/' + path);
 		}
 
 		if (relPath.endsWith('/..')) {
@@ -176,21 +182,5 @@ function normalize (path) {
 	if (slashed && path[0] !== '/') path = '/' + path;
 	else if (path.length === 0) path = '.';
 	return path;
-}
-
-function join () {
-	if (arguments.length === 0) return '.';
-	var joined;
-	for (var i = 0; i < arguments.length; ++i) {
-		var arg = arguments[i];
-		if (arg.length > 0) {
-		if (joined === undefined)
-			joined = arg;
-		else
-			joined += '/' + arg;
-		}
-	}
-	if (joined === undefined) return '.';
-	return joined;
 }`;
 }

packages/commonjs/src/index.js

@@ -1,7 +1,6 @@
-import { extname, relative } from 'path';
+import { extname, relative, resolve } from 'path';
 
 import { createFilter } from '@rollup/pluginutils';
-import getCommonDir from 'commondir';
 
 import { peerDependencies } from '../package.json';
 
@@ -64,13 +63,16 @@ export default function commonjs(options = {}) {
     getWrappedIds,
     isRequiredId
   } = getResolveRequireSourcesAndGetMeta(extensions, detectCyclesAndConditional);
-  const dynamicRequireModules = getDynamicRequireModules(options.dynamicRequireTargets);
-  const isDynamicRequireModulesEnabled = dynamicRequireModules.size > 0;
-  // TODO Lukas replace with new dynamicRequireRoot to replace CWD
+  const dynamicRequireRoot =
+    typeof options.dynamicRequireRoot === 'string'
+      ? resolve(options.dynamicRequireRoot)
+      : process.cwd();
   // TODO Lukas throw if require from outside commondir
-  const commonDir = isDynamicRequireModulesEnabled
-    ? getCommonDir(null, Array.from(dynamicRequireModules.keys()).concat(process.cwd()))
-    : null;
+  const { commonDir, dynamicRequireModules } = getDynamicRequireModules(
+    options.dynamicRequireTargets,
+    dynamicRequireRoot
+  );
+  const isDynamicRequireModulesEnabled = dynamicRequireModules.size > 0;
 
   const esModulesWithDefaultExport = new Set();
   const esModulesWithNamedExports = new Set();

packages/commonjs/src/utils.js

@@ -1,6 +1,6 @@
 /* eslint-disable import/prefer-default-export */
 
-import { basename, dirname, extname } from 'path';
+import { basename, dirname, extname, relative } from 'path';
 
 import { createFilter, makeLegalIdentifier } from '@rollup/pluginutils';
 
@@ -35,7 +35,7 @@ export function normalizePathSlashes(path) {
 }
 
 export const getVirtualPathForDynamicRequirePath = (path, commonDir) =>
-  normalizePathSlashes(path).slice(commonDir.length);
+  `/${normalizePathSlashes(relative(commonDir, path))}`;
 
 export function capitalize(name) {
   return name[0].toUpperCase() + name.slice(1);

packages/commonjs/test/fixtures/function/dynamic-require-root/_config.js

@@ -0,0 +1,7 @@
+module.exports = {
+  description: 'supports specifying a dynamic require root',
+  pluginOptions: {
+    dynamicRequireTargets: ['fixtures/function/dynamic-require-root/submodule.js'],
+    dynamicRequireRoot: 'fixtures/function/dynamic-require-root'
+  }
+};

packages/commonjs/test/fixtures/function/dynamic-require-root/main.js

@@ -0,0 +1,16 @@
+/* eslint-disable import/no-dynamic-require, global-require */
+
+let message;
+
+function takeModule(withName) {
+  return require(`./${withName}`);
+}
+
+try {
+  const submodule = takeModule('submodule');
+  message = submodule();
+} catch (err) {
+  ({ message } = err);
+}
+
+t.is(message, 'Hello there');

packages/commonjs/test/fixtures/function/dynamic-require-root/submodule.js

@@ -0,0 +1,3 @@
+module.exports = function () {
+  return 'Hello there';
+};