feat(commonjs): Basic support for circular dependencies

rollup · Dec 3, 2020 · f7f664d · f7f664d
1 parent 8c3a3a6
commit f7f664d
Show file tree

Hide file tree

Showing 57 changed files with 1,345 additions and 978 deletions.
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]) {

diff --git a/packages/commonjs/src/generate-exports.js b/packages/commonjs/src/generate-exports.js
@@ -1,12 +1,13 @@
-export function wrapCode(magicString, uses, moduleName, HELPERS_NAME, virtualDynamicRequirePath) {
+import { MODULE_SUFFIX, wrapId } from './helpers';
+
+export function wrapCode(magicString, uses, moduleName) {
   const args = `module${uses.exports ? ', exports' : ''}`;
+  const passedArgs = `${moduleName}${uses.exports ? `, ${moduleName}.exports` : ''}`;
 
   magicString
     .trim()
-    .prepend(`var ${moduleName} = ${HELPERS_NAME}.createCommonjsModule(function (${args}) {\n`)
-    .append(
-      `\n}${virtualDynamicRequirePath ? `, ${JSON.stringify(virtualDynamicRequirePath)}` : ''});`
-    );
+    .prepend(`(function (${args}) {\n`)
+    .append(`\n}(${passedArgs}));`);
 }
 
 export function rewriteExportsAndGetExportsBlock(
@@ -20,20 +21,30 @@ export function rewriteExportsAndGetExportsBlock(
   isRestorableCompiledEsm,
   code,
   uses,
-  HELPERS_NAME
+  HELPERS_NAME,
+  id
 ) {
-  const namedExportDeclarations = [`export { ${moduleName} as __moduleExports };`];
+  const exportDeclarations = [
+    `export { exports as __moduleExports } from ${JSON.stringify(wrapId(id, MODULE_SUFFIX))}`
+  ];
   const moduleExportsPropertyAssignments = [];
-  let deconflictedDefaultExportName;
 
-  if (!wrapped) {
-    let hasModuleExportsAssignment = false;
-    const namedExportProperties = [];
+  if (wrapped) {
+    if (defineCompiledEsmExpressions.length > 0 || code.indexOf('__esModule') >= 0) {
+      // eslint-disable-next-line no-param-reassign
+      uses.commonjsHelpers = true;
+      exportDeclarations.push(
+        `export default /*@__PURE__*/${HELPERS_NAME}.getDefaultExportFromCjs(${moduleName}.exports);`
+      );
+    } else {
+      exportDeclarations.push(`export default ${moduleName}.exports;`);
+    }
+  } else {
+    let deconflictedDefaultExportName;
 
     // Collect and rewrite module.exports assignments
     for (const { left } of topLevelModuleExportsAssignments) {
-      hasModuleExportsAssignment = true;
-      magicString.overwrite(left.start, left.end, `var ${moduleName}`);
+      magicString.overwrite(left.start, left.end, `${moduleName}.exports`);
     }
 
     // Collect and rewrite named exports
@@ -44,54 +55,35 @@ export function rewriteExportsAndGetExportsBlock(
       if (exportName === 'default') {
         deconflictedDefaultExportName = deconflicted;
       } else {
-        namedExportDeclarations.push(
+        exportDeclarations.push(
           exportName === deconflicted
             ? `export { ${exportName} };`
             : `export { ${deconflicted} as ${exportName} };`
         );
       }
 
-      if (hasModuleExportsAssignment) {
-        moduleExportsPropertyAssignments.push(`${moduleName}.${exportName} = ${deconflicted};`);
-      } else {
-        namedExportProperties.push(`\t${exportName}: ${deconflicted}`);
-      }
+      magicString.appendLeft(
+        code[node.end] === ';' ? node.end + 1 : node.end,
+        `\n${moduleName}.exports.${exportName} = ${deconflicted};`
+      );
     }
 
-    // Regenerate CommonJS namespace
-    if (!hasModuleExportsAssignment) {
-      const moduleExports = `{\n${namedExportProperties.join(',\n')}\n}`;
-      magicString
-        .trim()
-        .append(
-          `\n\nvar ${moduleName} = ${
-            isRestorableCompiledEsm
-              ? `/*#__PURE__*/Object.defineProperty(${moduleExports}, '__esModule', {value: true})`
-              : moduleExports
-          };`
-        );
+    if (isRestorableCompiledEsm) {
+      exportDeclarations.push(
+        deconflictedDefaultExportName
+          ? `export {${deconflictedDefaultExportName} as default};`
+          : `export default ${moduleName}.exports;`
+      );
+    } else if (deconflictedDefaultExportName && code.indexOf('__esModule') >= 0) {
+      // eslint-disable-next-line no-param-reassign
+      uses.commonjsHelpers = true;
+      exportDeclarations.push(
+        `export default /*@__PURE__*/${HELPERS_NAME}.getDefaultExportFromCjs(${moduleName}.exports);`
+      );
+    } else {
+      exportDeclarations.push(`export default ${moduleName}.exports;`);
     }
   }
 
-  // Generate default export
-  const defaultExport = [];
-  if (isRestorableCompiledEsm) {
-    defaultExport.push(`export default ${deconflictedDefaultExportName || moduleName};`);
-  } else if (
-    (wrapped || deconflictedDefaultExportName) &&
-    (defineCompiledEsmExpressions.length > 0 || code.indexOf('__esModule') >= 0)
-  ) {
-    // eslint-disable-next-line no-param-reassign
-    uses.commonjsHelpers = true;
-    defaultExport.push(
-      `export default /*@__PURE__*/${HELPERS_NAME}.getDefaultExportFromCjs(${moduleName});`
-    );
-  } else {
-    defaultExport.push(`export default ${moduleName};`);
-  }
-
-  return `\n\n${defaultExport
-    .concat(namedExportDeclarations)
-    .concat(moduleExportsPropertyAssignments)
-    .join('\n')}`;
+  return `\n\n${exportDeclarations.concat(moduleExportsPropertyAssignments).join('\n')}`;
 }
diff --git a/packages/commonjs/src/generate-imports.js b/packages/commonjs/src/generate-imports.js
@@ -3,7 +3,7 @@ import { dirname, resolve } from 'path';
 import { sync as nodeResolveSync } from 'resolve';
 
 import { isLocallyShadowed } from './ast-utils';
-import { HELPERS_ID, PROXY_SUFFIX, REQUIRE_SUFFIX, wrapId } from './helpers';
+import { MODULE_SUFFIX, HELPERS_ID, PROXY_SUFFIX, REQUIRE_SUFFIX, wrapId } from './helpers';
 import { normalizePathSlashes } from './utils';
 
 export function isRequireStatement(node, scope) {
@@ -122,7 +122,9 @@ export function getRequireHandlers() {
     topLevelRequireDeclarators,
     reassignedNames,
     helpersNameIfUsed,
-    dynamicRegisterSources
+    dynamicRegisterSources,
+    moduleName,
+    id
   ) {
     const removedDeclarators = getDeclaratorsReplacedByImportsAndSetImportNames(
       topLevelRequireDeclarators,
@@ -136,25 +138,30 @@ export function getRequireHandlers() {
     );
     removeDeclaratorsFromDeclarations(topLevelDeclarations, removedDeclarators, magicString);
     const importBlock = `${(helpersNameIfUsed
-      ? [`import * as ${helpersNameIfUsed} from '${HELPERS_ID}';`]
+      ? [`import * as ${helpersNameIfUsed} from "${HELPERS_ID}";`]
       : []
     )
+      .concat([
+        `import { __module as ${moduleName} } from ${JSON.stringify(wrapId(id, MODULE_SUFFIX))}`
+      ])
       .concat(
         // dynamic registers first, as the may be required in the other modules
-        [...dynamicRegisterSources].map((source) => `import '${wrapId(source, REQUIRE_SUFFIX)}';`),
+        [...dynamicRegisterSources].map(
+          (source) => `import ${JSON.stringify(wrapId(source, REQUIRE_SUFFIX))};`
+        ),
 
         // now the actual modules so that they are analyzed before creating the proxies;
         // no need to do this for virtual modules as we never proxy them
         requiredSources
           .filter((source) => !source.startsWith('\0'))
-          .map((source) => `import '${wrapId(source, REQUIRE_SUFFIX)}';`),
+          .map((source) => `import ${JSON.stringify(wrapId(source, REQUIRE_SUFFIX))};`),
 
         // now the proxy modules
         requiredSources.map((source) => {
           const { name, nodesUsingRequired } = requiredBySource[source];
-          return `import ${nodesUsingRequired.length ? `${name} from ` : ``}'${
+          return `import ${nodesUsingRequired.length ? `${name} from ` : ''}${JSON.stringify(
             source.startsWith('\0') ? source : wrapId(source, PROXY_SUFFIX)
-          }';`;
+          )};`;
         })
       )
       .join('\n')}`;

diff --git a/packages/commonjs/src/helpers.js b/packages/commonjs/src/helpers.js
@@ -5,6 +5,8 @@ export const unwrapId = (wrappedId, suffix) => wrappedId.slice(1, -suffix.length
 export const PROXY_SUFFIX = '?commonjs-proxy';
 export const REQUIRE_SUFFIX = '?commonjs-require';
 export const EXTERNAL_SUFFIX = '?commonjs-external';
+// export const EXPORTS_SUFFIX = '?commonjs-exports';
+export const MODULE_SUFFIX = '?commonjs-module';
 
 export const DYNAMIC_REGISTER_PREFIX = '\0commonjs-dynamic-register:';
 export const DYNAMIC_JSON_PREFIX = '\0commonjs-dynamic-json:';
@@ -49,25 +51,20 @@ export function getAugmentedNamespace(n) {
 `;
 
 const HELPER_NON_DYNAMIC = `
-export function createCommonjsModule(fn) {
-  var module = { exports: {} }
-	return fn(module, module.exports), module.exports;
-}
-
 export function commonjsRequire (target) {
 	throw new Error('Could not dynamically require "' + target + '". Please configure the dynamicRequireTargets option of @rollup/plugin-commonjs appropriately for this require call to behave properly.');
 }
 `;
 
 const HELPERS_DYNAMIC = `
-export function createCommonjsModule(fn, basedir, module) {
-	return module = {
-		path: basedir,
+export function createModule(modulePath) {
+	return {
+		path: modulePath,
 		exports: {},
 		require: function (path, base) {
-			return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
+			return commonjsRequire(path, base == null ? modulePath : base);
 		}
-	}, fn(module, module.exports), module.exports;
+	};
 }
 
 export function commonjsRegister (path, loader) {

diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js
@@ -1,4 +1,4 @@
-import { extname } from 'path';
+import { dirname, extname } from 'path';
 
 import { createFilter } from '@rollup/pluginutils';
 import getCommonDir from 'commondir';
@@ -20,6 +20,7 @@ import {
   getHelpersModule,
   HELPERS_ID,
   isWrappedId,
+  MODULE_SUFFIX,
   PROXY_SUFFIX,
   unwrapId
 } from './helpers';
@@ -35,7 +36,7 @@ import {
 import getResolveId from './resolve-id';
 import validateRollupVersion from './rollup-version';
 import transformCommonjs from './transform-commonjs';
-import { normalizePathSlashes } from './utils';
+import { getName, getVirtualPathForDynamicRequirePath, normalizePathSlashes } from './utils';
 
 export default function commonjs(options = {}) {
   const extensions = options.extensions || ['.js'];
@@ -81,6 +82,7 @@ export default function commonjs(options = {}) {
 
   function transformAndCheckExports(code, id) {
     if (isDynamicRequireModulesEnabled && this.getModuleInfo(id).isEntry) {
+      // eslint-disable-next-line no-param-reassign
       code =
         getDynamicPackagesEntryIntro(dynamicRequireModuleDirPaths, dynamicRequireModuleSet) + code;
     }
@@ -104,7 +106,7 @@ export default function commonjs(options = {}) {
       return { meta: { commonjs: { isCommonJS: false } } };
     }
 
-    // avoid wrapping in createCommonjsModule, as this is a commonjsRegister call
+    // avoid wrapping as this is a commonjsRegister call
     const disableWrap = isModuleRegistrationProxy(id, dynamicRequireModuleSet);
 
     return transformCommonjs(
@@ -137,6 +139,16 @@ export default function commonjs(options = {}) {
 
     resolveId,
 
+    // TODO Lukas in Rollup, ensure synthetic namespace is only rendered when needed
+    // TODO Lukas
+    //  - Only wrap if
+    //    - there is an assignment to module.exports (also check destructuring) or
+    //    - unchecked usages of module or
+    //    - direct eassignment to exports (also check destructuring)
+    //  - Use foo?exports instead of foo?module if there are no assignments to module.exports
+    //    (also check destructring)
+    //  - Do not use foo?module and do not wrap if there are only direct top-level module.exports
+    //    assignments and no exports property assignments
     load(id) {
       if (id === HELPERS_ID) {
         return getHelpersModule(isDynamicRequireModulesEnabled);
@@ -146,6 +158,30 @@ export default function commonjs(options = {}) {
         return getSpecificHelperProxy(id);
       }
 
+      if (isWrappedId(id, MODULE_SUFFIX)) {
+        const actualId = unwrapId(id, MODULE_SUFFIX);
+        let name = getName(actualId);
+        let code;
+        if (isDynamicRequireModulesEnabled) {
+          if (['modulePath', 'commonjsRequire', 'createModule'].includes(name)) {
+            name = `${name}_`;
+          }
+          code =
+            `import {commonjsRequire, createModule} from "${HELPERS_ID}";\n` +
+            `var ${name} = createModule(${JSON.stringify(
+              getVirtualPathForDynamicRequirePath(dirname(actualId), commonDir)
+            )});\n` +
+            `export {${name} as __module}`;
+        } else {
+          code = `var ${name} = {exports: {}}; export {${name} as __module}`;
+        }
+        return {
+          code,
+          syntheticNamedExports: '__module',
+          meta: { commonjs: { isCommonJS: false } }
+        };
+      }
+
       if (isWrappedId(id, EXTERNAL_SUFFIX)) {
         const actualId = unwrapId(id, EXTERNAL_SUFFIX);
         return getUnknownRequireProxy(
@@ -197,9 +233,9 @@ export default function commonjs(options = {}) {
       }
     },
 
-    moduleParsed({ id, meta: { commonjs } }) {
-      if (commonjs) {
-        const isCjs = commonjs.isCommonJS;
+    moduleParsed({ id, meta: { commonjs: commonjsMeta } }) {
+      if (commonjsMeta) {
+        const isCjs = commonjsMeta.isCommonJS;
         if (isCjs != null) {
           setIsCjsPromise(id, isCjs);
           return;

diff --git a/packages/commonjs/src/proxies.js b/packages/commonjs/src/proxies.js
@@ -1,12 +1,12 @@
 import { readFileSync } from 'fs';
 
-import { DYNAMIC_JSON_PREFIX, HELPERS_ID } from './helpers';
+import { DYNAMIC_JSON_PREFIX, MODULE_SUFFIX, HELPERS_ID, wrapId } from './helpers';
 import { getIsCjsPromise } from './is-cjs';
 import { getName, getVirtualPathForDynamicRequirePath, normalizePathSlashes } from './utils';
 
 // e.g. id === "commonjsHelpers?commonjsRegister"
 export function getSpecificHelperProxy(id) {
-  return `export {${id.split('?')[1]} as default} from '${HELPERS_ID}';`;
+  return `export {${id.split('?')[1]} as default} from "${HELPERS_ID}";`;
 }
 
 export function getUnknownRequireProxy(id, requireReturnsDefault) {
@@ -51,7 +51,7 @@ export async function getStaticRequireProxy(
   const name = getName(id);
   const isCjs = await getIsCjsPromise(id);
   if (isCjs) {
-    return `import { __moduleExports } from ${JSON.stringify(id)}; export default __moduleExports;`;
+    return `export { exports as default } from ${JSON.stringify(wrapId(id, MODULE_SUFFIX))};`;
   } else if (isCjs === null) {
     return getUnknownRequireProxy(id, requireReturnsDefault);
   } else if (!requireReturnsDefault) {

diff --git a/packages/commonjs/src/resolve-id.js b/packages/commonjs/src/resolve-id.js
@@ -6,6 +6,7 @@ import { dirname, resolve, sep } from 'path';
 import {
   DYNAMIC_JSON_PREFIX,
   DYNAMIC_PACKAGES_ID,
+  MODULE_SUFFIX,
   EXTERNAL_SUFFIX,
   HELPERS_ID,
   isWrappedId,
@@ -47,7 +48,11 @@ export default function getResolveId(extensions) {
   }
 
   return function resolveId(importee, importer) {
-    // Proxies are only importing resolved ids, no need to resolve again
+    if (isWrappedId(importee, MODULE_SUFFIX)) {
+      return importee;
+    }
+    // Except for exports, proxies are only importing resolved ids,
+    // no need to resolve again
     if (importer && isWrappedId(importer, PROXY_SUFFIX)) {
       return importee;
     }