feat(commonjs): Generate simpler code when replacing module.exports

rollup · Nov 25, 2020 · ec20e09 · ec20e09
1 parent e8948a0
commit ec20e09
Show file tree

Hide file tree

Showing 31 changed files with 458 additions and 704 deletions.
diff --git a/packages/commonjs/src/generate-exports.js b/packages/commonjs/src/generate-exports.js
@@ -21,81 +21,104 @@ export function rewriteExportsAndGetExportsBlock(
   code,
   uses,
   HELPERS_NAME,
-  id
+  id,
+  replacesModuleExports
 ) {
-  const exportDeclarations = [
-    `export { exports as __moduleExports } from ${JSON.stringify(wrapId(id, MODULE_SUFFIX))}`
-  ];
-  const moduleExportsPropertyAssignments = [];
+  const exportDeclarations = [];
 
-  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);`
-      );
+  // TODO Lukas maybe introduce an export mode with
+  //  - 'replace'
+  //  - 'wrapped'
+  //  - 'module'
+  //  - 'exports'
+  //  then consider extracting parts that "getExportDeclarations"
+  if (replacesModuleExports) {
+    if (topLevelModuleExportsAssignments.length > 0) {
+      for (const { left } of topLevelModuleExportsAssignments) {
+        magicString.overwrite(left.start, left.end, `var ${moduleName}`);
+      }
     } else {
-      exportDeclarations.push(`export default ${moduleName}.exports;`);
+      exportDeclarations.unshift(`var ${moduleName} = {};`);
     }
+    exportDeclarations.push(
+      `export { ${moduleName} as __moduleExports };`,
+      `export { ${moduleName} as default };`
+    );
   } else {
-    let deconflictedDefaultExportName;
+    exportDeclarations.push(
+      `export { exports as __moduleExports } from ${JSON.stringify(wrapId(id, MODULE_SUFFIX))}`
+    );
+    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) {
-      magicString.overwrite(left.start, left.end, `${moduleName}.exports`);
-    }
+      // TODO Lukas if we have an assignment to module.exports and assignments to exports.foo or usages of exports we need to wrap
+      // TODO Lukas remove?
+      // Collect and rewrite module.exports assignments
+      for (const { left } of topLevelModuleExportsAssignments) {
+        magicString.overwrite(left.start, left.end, `${moduleName}.exports`);
+      }
 
-    // Collect and rewrite named exports
-    for (const [exportName, node] of topLevelExportsAssignmentsByName) {
-      const deconflicted = deconflict(exportName);
-      magicString.overwrite(node.start, node.left.end, `var ${deconflicted}`);
+      // Collect and rewrite named exports
+      for (const [exportName, node] of topLevelExportsAssignmentsByName) {
+        const deconflicted = deconflict(exportName);
+        magicString.overwrite(node.start, node.left.end, `var ${deconflicted}`);
 
-      if (exportName === 'default') {
-        deconflictedDefaultExportName = deconflicted;
-      } else {
-        exportDeclarations.push(
-          exportName === deconflicted
-            ? `export { ${exportName} };`
-            : `export { ${deconflicted} as ${exportName} };`
+        if (exportName === 'default') {
+          deconflictedDefaultExportName = deconflicted;
+        } else {
+          exportDeclarations.push(
+            exportName === deconflicted
+              ? `export { ${exportName} };`
+              : `export { ${deconflicted} as ${exportName} };`
+          );
+        }
+
+        magicString.appendLeft(
+          code[node.end] === ';' ? node.end + 1 : node.end,
+          `\n${moduleName}.exports.${exportName} = ${deconflicted};`
         );
       }
 
-      magicString.appendLeft(
-        code[node.end] === ';' ? node.end + 1 : node.end,
-        `\n${moduleName}.exports.${exportName} = ${deconflicted};`
-      );
-    }
-
-    // Collect and rewrite exports.__esModule assignments
-    let isRestorableCompiledEsm = false;
-    for (const expression of defineCompiledEsmExpressions) {
-      isRestorableCompiledEsm = true;
-      const moduleExportsExpression =
-        expression.type === 'CallExpression' ? expression.arguments[0] : expression.left.object;
-      magicString.overwrite(
-        moduleExportsExpression.start,
-        moduleExportsExpression.end,
-        `${moduleName}.exports`
-      );
-    }
+      // Collect and rewrite exports.__esModule assignments
+      let isRestorableCompiledEsm = false;
+      for (const expression of defineCompiledEsmExpressions) {
+        isRestorableCompiledEsm = true;
+        const moduleExportsExpression =
+          expression.type === 'CallExpression' ? expression.arguments[0] : expression.left.object;
+        magicString.overwrite(
+          moduleExportsExpression.start,
+          moduleExportsExpression.end,
+          `${moduleName}.exports`
+        );
+      }
 
-    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;`);
+      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;`);
+      }
     }
   }
 
-  return `\n\n${exportDeclarations.concat(moduleExportsPropertyAssignments).join('\n')}`;
+  return `\n\n${exportDeclarations.join('\n')}`;
 }
diff --git a/packages/commonjs/src/generate-imports.js b/packages/commonjs/src/generate-imports.js
@@ -124,7 +124,8 @@ export function getRequireHandlers() {
     helpersNameIfUsed,
     dynamicRegisterSources,
     moduleName,
-    id
+    id,
+    replacesModuleExports
   ) {
     const removedDeclarators = getDeclaratorsReplacedByImportsAndSetImportNames(
       topLevelRequireDeclarators,
@@ -141,9 +142,15 @@ export function getRequireHandlers() {
       ? [`import * as ${helpersNameIfUsed} from "${HELPERS_ID}";`]
       : []
     )
-      .concat([
-        `import { __module as ${moduleName} } from ${JSON.stringify(wrapId(id, MODULE_SUFFIX))}`
-      ])
+      .concat(
+        replacesModuleExports
+          ? []
+          : [
+              `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(

diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js
@@ -24,7 +24,7 @@ import {
   PROXY_SUFFIX,
   unwrapId
 } from './helpers';
-import { setIsCjsPromise } from './is-cjs';
+import { setCommonJSMetaPromise } from './is-cjs';
 import { hasCjsKeywords } from './parse';
 import {
   getDynamicJsonProxy,
@@ -141,14 +141,12 @@ export default function commonjs(options = {}) {
 
     // TODO Lukas in Rollup, ensure synthetic namespace is only rendered when needed
     // TODO Lukas
+    //  - Use foo?exports instead of foo?module if there are no assignments to module.exports
     //  - 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);
@@ -234,14 +232,11 @@ export default function commonjs(options = {}) {
     },
 
     moduleParsed({ id, meta: { commonjs: commonjsMeta } }) {
-      if (commonjsMeta) {
-        const isCjs = commonjsMeta.isCommonJS;
-        if (isCjs != null) {
-          setIsCjsPromise(id, isCjs);
-          return;
-        }
+      if (commonjsMeta && commonjsMeta.isCommonJS != null) {
+        setCommonJSMetaPromise(id, commonjsMeta);
+        return;
       }
-      setIsCjsPromise(id, null);
+      setCommonJSMetaPromise(id, null);
     }
   };
 }
diff --git a/packages/commonjs/src/is-cjs.js b/packages/commonjs/src/is-cjs.js
@@ -1,29 +1,29 @@
-const isCjsPromises = new Map();
+const commonJSMetaPromises = new Map();
 
-export function getIsCjsPromise(id) {
-  let isCjsPromise = isCjsPromises.get(id);
-  if (isCjsPromise) return isCjsPromise.promise;
+export function getCommonJSMetaPromise(id) {
+  let commonJSMetaPromise = commonJSMetaPromises.get(id);
+  if (commonJSMetaPromise) return commonJSMetaPromise.promise;
 
   const promise = new Promise((resolve) => {
-    isCjsPromise = {
+    commonJSMetaPromise = {
       resolve,
       promise: null
     };
-    isCjsPromises.set(id, isCjsPromise);
+    commonJSMetaPromises.set(id, commonJSMetaPromise);
   });
-  isCjsPromise.promise = promise;
+  commonJSMetaPromise.promise = promise;
 
   return promise;
 }
 
-export function setIsCjsPromise(id, resolution) {
-  const isCjsPromise = isCjsPromises.get(id);
-  if (isCjsPromise) {
-    if (isCjsPromise.resolve) {
-      isCjsPromise.resolve(resolution);
-      isCjsPromise.resolve = null;
+export function setCommonJSMetaPromise(id, commonjsMeta) {
+  const commonJSMetaPromise = commonJSMetaPromises.get(id);
+  if (commonJSMetaPromise) {
+    if (commonJSMetaPromise.resolve) {
+      commonJSMetaPromise.resolve(commonjsMeta);
+      commonJSMetaPromise.resolve = null;
     }
   } else {
-    isCjsPromises.set(id, { promise: Promise.resolve(resolution), resolve: null });
+    commonJSMetaPromises.set(id, { promise: Promise.resolve(commonjsMeta), resolve: null });
   }
 }
diff --git a/packages/commonjs/src/proxies.js b/packages/commonjs/src/proxies.js
@@ -1,7 +1,7 @@
 import { readFileSync } from 'fs';
 
-import { DYNAMIC_JSON_PREFIX, MODULE_SUFFIX, HELPERS_ID, wrapId } from './helpers';
-import { getIsCjsPromise } from './is-cjs';
+import { DYNAMIC_JSON_PREFIX, HELPERS_ID } from './helpers';
+import { getCommonJSMetaPromise } from './is-cjs';
 import { getName, getVirtualPathForDynamicRequirePath, normalizePathSlashes } from './utils';
 
 // e.g. id === "commonjsHelpers?commonjsRegister"
@@ -49,13 +49,13 @@ export async function getStaticRequireProxy(
   esModulesWithNamedExports
 ) {
   const name = getName(id);
-  const isCjs = await getIsCjsPromise(id);
-  if (isCjs) {
-    return `export { exports as default } from ${JSON.stringify(wrapId(id, MODULE_SUFFIX))};`;
-  } else if (isCjs === null) {
+  const commonjsMeta = await getCommonJSMetaPromise(id);
+  if (commonjsMeta && commonjsMeta.isCommonJS) {
+    return `export { __moduleExports as default } from ${JSON.stringify(id)};`;
+  } else if (commonjsMeta === null) {
     return getUnknownRequireProxy(id, requireReturnsDefault);
   } else if (!requireReturnsDefault) {
-    return `import {getAugmentedNamespace} from "${HELPERS_ID}"; import * as ${name} from ${JSON.stringify(
+    return `import { getAugmentedNamespace } from "${HELPERS_ID}"; import * as ${name} from ${JSON.stringify(
       id
     )}; export default /*@__PURE__*/getAugmentedNamespace(${name});`;
   } else if (
@@ -66,5 +66,5 @@ export async function getStaticRequireProxy(
   ) {
     return `import * as ${name} from ${JSON.stringify(id)}; export default ${name};`;
   }
-  return `export {default} from ${JSON.stringify(id)};`;
+  return `export { default } from ${JSON.stringify(id)};`;
 }
diff --git a/packages/commonjs/src/transform-commonjs.js b/packages/commonjs/src/transform-commonjs.js
@@ -349,6 +349,24 @@ export default function transformCommonjs(
     magicString.remove(0, commentEnd).trim();
   }
 
+  // TODO Lukas test all cases
+  const replacesModuleExports =
+    !shouldWrap &&
+    topLevelExportsAssignmentsByName.size === 0 &&
+    defineCompiledEsmExpressions.length === 0;
+
+  const importBlock = rewriteRequireExpressionsAndGetImportBlock(
+    magicString,
+    topLevelDeclarations,
+    topLevelRequireDeclarators,
+    reassignedNames,
+    uses.commonjsHelpers && HELPERS_NAME,
+    dynamicRegisterSources,
+    moduleName,
+    id,
+    replacesModuleExports
+  );
+
   const exportBlock = isEsModule
     ? ''
     : rewriteExportsAndGetExportsBlock(
@@ -362,20 +380,10 @@ export default function transformCommonjs(
         code,
         uses,
         HELPERS_NAME,
-        id
+        id,
+        replacesModuleExports
       );
 
-  const importBlock = rewriteRequireExpressionsAndGetImportBlock(
-    magicString,
-    topLevelDeclarations,
-    topLevelRequireDeclarators,
-    reassignedNames,
-    uses.commonjsHelpers && HELPERS_NAME,
-    dynamicRegisterSources,
-    moduleName,
-    id
-  );
-
   if (shouldWrap) {
     wrapCode(magicString, uses, moduleName, HELPERS_NAME, virtualDynamicRequirePath);
   }

diff --git a/packages/commonjs/test/fixtures/form/async-function/output.js b/packages/commonjs/test/fixtures/form/async-function/output.js
@@ -1,8 +1,6 @@
-import { __module as input } from "\u0000fixtures/form/async-function/input.js?commonjs-module"
-
-input.exports = async function () {
+var input = async function () {
 	// TODO
 };
 
-export { exports as __moduleExports } from "\u0000fixtures/form/async-function/input.js?commonjs-module"
-export default input.exports;
+export { input as __moduleExports };
+export { input as default };
diff --git a/packages/commonjs/test/fixtures/form/constant-template-literal/output.js b/packages/commonjs/test/fixtures/form/constant-template-literal/output.js
@@ -1,8 +1,8 @@
-import { __module as input } from "\u0000fixtures/form/constant-template-literal/input.js?commonjs-module"
 import "\u0000tape?commonjs-require";
 import foo from "\u0000tape?commonjs-proxy";
 
 console.log(foo);
 
-export { exports as __moduleExports } from "\u0000fixtures/form/constant-template-literal/input.js?commonjs-module"
-export default input.exports;
+var input = {};
+export { input as __moduleExports };
+export { input as default };