feat(commonjs)!: Add support for circular dependencies (#658)

* feat(commonjs): Basic support for circular dependencies * refactor(commonjs): Inline __esModule handling into export generation * feat(commonjs): Generate simpler code when replacing module.exports * feat(commonjs): Handle side-effect free modules with updated Rollup * feat(commonjs): Generate simplified code when module is not used * feat(commonjs): Do not use module when wrapping if not needed BREAKING CHANGES: now requires at least Rollup 2.38.0 * chore(commonjs): Clean up usage of helpers * feat(commonjs): Inline name export assignments * feat(commonjs): Do not wrap for nested module.exports assignments * feat(commonjs): Wrap if accessing exports while reassigning module.exports * chore(commonjs): Add test for module.exports mutations * feat(commonjs): Do not wrap for double exports assignments * fix(commonjs): Deconflict nested module.exports assignments * chore(commonjs): Simplify assignment tracking * feat(commonjs): Do not wrap for nested named export assignments * chore(commonjs): Clean up remaining TODOs * fix(commonjs): Make sure require expressions return snapshots * fix(commonjs): Snapshot default export * chore(commonjs): Unwrapped wrapper helper functions * chore: update dependencies * chore: change audit level to high amid unfixable moderate Co-authored-by: Daniel Cohen Gindi <[email protected]> Co-authored-by: shellscape <[email protected]>
rollup · May 7, 2021 · 5fe760b · 5fe760b
1 parent 9d9b359
commit 5fe760b
Show file tree

Hide file tree

Showing 127 changed files with 6,302 additions and 7,354 deletions.
diff --git a/package.json b/package.json
@@ -11,7 +11,7 @@
     "preinstall": "node scripts/disallow-npm.js",
     "pub": "node scripts/pub.js",
     "publish": "node scripts/publish.js",
-    "security": "pnpm audit --audit-level=moderate"
+    "security": "pnpm audit --audit-level=high"
   },
   "dependencies": {
     "conventional-commits-parser": "^3.1.0",

diff --git a/packages/commonjs/README.md b/packages/commonjs/README.md
@@ -34,9 +34,9 @@ export default {
   input: 'src/index.js',
   output: {
     dir: 'output',
-    format: 'cjs'
+    format: 'cjs',
   },
-  plugins: [commonjs()]
+  plugins: [commonjs()],
 };
 ```
 
@@ -66,8 +66,8 @@ commonjs({
     '!node_modules/logform/index.js',
     '!node_modules/logform/format.js',
     '!node_modules/logform/levels.js',
-    '!node_modules/logform/browser.js'
-  ]
+    '!node_modules/logform/browser.js',
+  ],
 });
 ```
 
@@ -250,7 +250,7 @@ This is in line with how other bundlers handle this situation and is also the mo
 
   var dep$1 = /*#__PURE__*/ Object.freeze({
     __proto__: null,
-    default: dep
+    default: dep,
   });
 
   console.log(dep$1.default);
@@ -281,7 +281,7 @@ For these situations, you can change Rollup's behaviour either globally or per m
               enumerable: true,
               get: function () {
                 return n[k];
-              }
+              },
             }
       );
     });
@@ -357,9 +357,9 @@ export default {
   output: {
     file: 'bundle.js',
     format: 'iife',
-    name: 'MyModule'
+    name: 'MyModule',
   },
-  plugins: [resolve(), commonjs()]
+  plugins: [resolve(), commonjs()],
 };
 ```
 
@@ -369,7 +369,7 @@ Symlinks are common in monorepos and are also created by the `npm link` command.
 
 ```js
 commonjs({
-  include: /node_modules/
+  include: /node_modules/,
 });
 ```
 
@@ -392,11 +392,11 @@ function cjsDetectionPlugin() {
     moduleParsed({
       id,
       meta: {
-        commonjs: { isCommonJS }
-      }
+        commonjs: { isCommonJS },
+      },
     }) {
       console.log(`File ${id} is CommonJS: ${isCommonJS}`);
-    }
+    },
   };
 }
 ```

diff --git a/packages/commonjs/package.json b/packages/commonjs/package.json
@@ -46,7 +46,7 @@
     "require"
   ],
   "peerDependencies": {
-    "rollup": "^2.30.0"
+    "rollup": "^2.38.3"
   },
   "dependencies": {
     "@rollup/pluginutils": "^3.1.0",
@@ -62,7 +62,7 @@
     "@rollup/plugin-node-resolve": "^8.4.0",
     "locate-character": "^2.0.5",
     "require-relative": "^0.8.7",
-    "rollup": "^2.30.0",
+    "rollup": "^2.39.0",
     "shx": "^0.3.2",
     "source-map": "^0.7.3",
     "source-map-support": "^0.5.19",

diff --git a/packages/commonjs/src/ast-utils.js b/packages/commonjs/src/ast-utils.js
@@ -70,15 +70,14 @@ export function isDefineCompiledEsm(node) {
 }
 
 function getDefinePropertyCallName(node, targetName) {
-  const targetNames = targetName.split('.');
-
   const {
     callee: { object, property }
   } = node;
   if (!object || object.type !== 'Identifier' || object.name !== 'Object') return;
   if (!property || property.type !== 'Identifier' || property.name !== 'defineProperty') return;
   if (node.arguments.length !== 3) return;
 
+  const targetNames = targetName.split('.');
   const [target, key, value] = node.arguments;
   if (targetNames.length === 1) {
     if (target.type !== 'Identifier' || target.name !== targetNames[0]) {
@@ -105,17 +104,6 @@ function getDefinePropertyCallName(node, targetName) {
   return { key: key.value, value: valueProperty.value };
 }
 
-export function isLocallyShadowed(name, scope) {
-  while (scope.parent) {
-    if (scope.declarations[name]) {
-      return true;
-    }
-    // eslint-disable-next-line no-param-reassign
-    scope = scope.parent;
-  }
-  return false;
-}
-
 export function isShorthandProperty(parent) {
   return parent && parent.type === 'Property' && parent.shorthand;
 }
diff --git a/packages/commonjs/src/dynamic-packages-manager.js b/packages/commonjs/src/dynamic-packages-manager.js
@@ -1,14 +1,7 @@
 import { existsSync, readFileSync } from 'fs';
 import { join } from 'path';
 
-import {
-  DYNAMIC_PACKAGES_ID,
-  DYNAMIC_REGISTER_SUFFIX,
-  HELPERS_ID,
-  isWrappedId,
-  unwrapId,
-  wrapId
-} from './helpers';
+import { DYNAMIC_PACKAGES_ID, DYNAMIC_REGISTER_SUFFIX, HELPERS_ID, wrapId } from './helpers';
 import { getVirtualPathForDynamicRequirePath, normalizePathSlashes } from './utils';
 
 export function getPackageEntryPoint(dirPath) {
@@ -47,28 +40,18 @@ export function getDynamicPackagesEntryIntro(
 ) {
   let dynamicImports = Array.from(
     dynamicRequireModuleSet,
-    (dynamicId) => `require(${JSON.stringify(wrapModuleRegisterProxy(dynamicId))});`
+    (dynamicId) => `require(${JSON.stringify(wrapId(dynamicId, DYNAMIC_REGISTER_SUFFIX))});`
   ).join('\n');
 
   if (dynamicRequireModuleDirPaths.length) {
-    dynamicImports += `require(${JSON.stringify(wrapModuleRegisterProxy(DYNAMIC_PACKAGES_ID))});`;
+    dynamicImports += `require(${JSON.stringify(
+      wrapId(DYNAMIC_PACKAGES_ID, DYNAMIC_REGISTER_SUFFIX)
+    )});`;
   }
 
   return dynamicImports;
 }
 
-export function wrapModuleRegisterProxy(id) {
-  return wrapId(id, DYNAMIC_REGISTER_SUFFIX);
-}
-
-export function unwrapModuleRegisterProxy(id) {
-  return unwrapId(id, DYNAMIC_REGISTER_SUFFIX);
-}
-
-export function isModuleRegisterProxy(id) {
-  return isWrappedId(id, DYNAMIC_REGISTER_SUFFIX);
-}
-
 export function isDynamicModuleImport(id, dynamicRequireModuleSet) {
   const normalizedPath = normalizePathSlashes(id);
   return dynamicRequireModuleSet.has(normalizedPath) && !normalizedPath.endsWith('.json');