feat(commonjs): support remaining export modes

packages/commonjs/src/generate-exports.js

@@ -11,8 +11,9 @@ export function wrapCode(magicString, uses, moduleName, exportsName) {
   }
   magicString
     .trim()
+    .indent('\t')
     .prepend(`(function (${args.join(', ')}) {\n`)
-    .append(`\n}(${passedArgs.join(', ')}));`);
+    .append(`\n} (${passedArgs.join(', ')}));`);
 }
 
 export function rewriteExportsAndGetExportsBlock(
@@ -38,34 +39,18 @@ export function rewriteExportsAndGetExportsBlock(
   const exportDeclarations = [];
 
   if (usesRequireWrapper) {
-    // TODO Lukas Extract
-    if (exportMode === 'replace') {
-      for (const { left } of moduleExportsAssignments) {
-        magicString.overwrite(left.start, left.end, exportsName);
-      }
-    } else {
-      // Collect and rewrite module.exports assignments
-      for (const { left } of moduleExportsAssignments) {
-        magicString.overwrite(left.start, left.end, `${moduleName}.exports`);
-      }
-      // Collect and rewrite named exports
-      for (const [exportName, { nodes }] of exportsAssignmentsByName) {
-        for (const node of nodes) {
-          magicString.overwrite(node.start, node.left.end, `${exportsName}.${exportName}`);
-        }
-      }
-      // Collect and rewrite exports.__esModule assignments
-      for (const expression of defineCompiledEsmExpressions) {
-        const moduleExportsExpression =
-          expression.type === 'CallExpression' ? expression.arguments[0] : expression.left.object;
-        magicString.overwrite(
-          moduleExportsExpression.start,
-          moduleExportsExpression.end,
-          exportsName
-        );
-      }
-    }
-    exports.push(`${requireName} as __require`);
+    getExportsWhenUsingRequireWrapper(
+      magicString,
+      wrapped,
+      exportMode,
+      exports,
+      moduleExportsAssignments,
+      exportsAssignmentsByName,
+      moduleName,
+      exportsName,
+      requireName,
+      defineCompiledEsmExpressions
+    );
   } else if (exportMode === 'replace') {
     getExportsForReplacedModuleExports(
       magicString,
@@ -109,6 +94,49 @@ export function rewriteExportsAndGetExportsBlock(
   return `\n\n${exportDeclarations.join('\n')}`;
 }
 
+function getExportsWhenUsingRequireWrapper(
+  magicString,
+  wrapped,
+  exportMode,
+  exports,
+  moduleExportsAssignments,
+  exportsAssignmentsByName,
+  moduleName,
+  exportsName,
+  requireName,
+  defineCompiledEsmExpressions
+) {
+  if (!wrapped) {
+    if (exportMode === 'replace') {
+      for (const { left } of moduleExportsAssignments) {
+        magicString.overwrite(left.start, left.end, exportsName);
+      }
+    } else {
+      // Collect and rewrite module.exports assignments
+      for (const { left } of moduleExportsAssignments) {
+        magicString.overwrite(left.start, left.end, `${moduleName}.exports`);
+      }
+      // Collect and rewrite named exports
+      for (const [exportName, { nodes }] of exportsAssignmentsByName) {
+        for (const node of nodes) {
+          magicString.overwrite(node.start, node.left.end, `${exportsName}.${exportName}`);
+        }
+      }
+      // Collect and rewrite exports.__esModule assignments
+      for (const expression of defineCompiledEsmExpressions) {
+        const moduleExportsExpression =
+          expression.type === 'CallExpression' ? expression.arguments[0] : expression.left.object;
+        magicString.overwrite(
+          moduleExportsExpression.start,
+          moduleExportsExpression.end,
+          exportsName
+        );
+      }
+    }
+  }
+  exports.push(`${requireName} as __require`);
+}
+
 function getExportsForReplacedModuleExports(
   magicString,
   exports,
@@ -196,7 +224,6 @@ function getExports(
   }
 
   if (!isRestorableCompiledEsm || defaultIsModuleExports === true) {
-    // TODO Lukas handle ESM importing CommonJS
     exports.push(`${exportsName} as default`);
   } else if (moduleExportsAssignments.length === 0 || defaultIsModuleExports === false) {
     exports.push(`${deconflictedDefaultExportName || exportsName} as default`);

packages/commonjs/src/generate-imports.js

@@ -2,7 +2,14 @@ import { dirname, resolve } from 'path';
 
 import { sync as nodeResolveSync } from 'resolve';
 
-import { DYNAMIC_MODULES_ID, EXPORTS_SUFFIX, HELPERS_ID, MODULE_SUFFIX, wrapId } from './helpers';
+import {
+  DYNAMIC_MODULES_ID,
+  EXPORTS_SUFFIX,
+  HELPERS_ID,
+  IS_WRAPPED_COMMONJS,
+  MODULE_SUFFIX,
+  wrapId
+} from './helpers';
 import { normalizePathSlashes } from './utils';
 
 export function isRequireStatement(node, scope) {
@@ -129,7 +136,7 @@ export function getRequireHandlers() {
     const requiresBySource = collectSources(requireExpressions);
     // TODO Lukas consider extracting stuff
     const result = await resolveRequireSourcesAndGetMeta(
-      usesRequireWrapper ? 'withRequireFunction' : true,
+      usesRequireWrapper ? IS_WRAPPED_COMMONJS : true,
       Object.keys(requiresBySource)
     );
     let uid = 0;
@@ -142,9 +149,7 @@ export function getRequireHandlers() {
         name = `require$$${uid}`;
         uid += 1;
       } while (requires.some(hasNameConflict));
-
-      // TODO Lukas extract constant
-      if (isCommonJS === 'withRequireFunction') {
+      if (isCommonJS === IS_WRAPPED_COMMONJS) {
         for (const { node } of requires) {
           magicString.overwrite(node.start, node.end, `${name}()`);
         }

packages/commonjs/src/helpers.js

@@ -11,6 +11,8 @@ export const ES_IMPORT_SUFFIX = '?es-import';
 export const DYNAMIC_MODULES_ID = '\0commonjs-dynamic-modules';
 export const HELPERS_ID = '\0commonjsHelpers.js';
 
+export const IS_WRAPPED_COMMONJS = 'withRequireFunction';
+
 // `x['default']` is used instead of `x.default` for backward compatibility with ES3 browsers.
 // Minifiers like uglify will usually transpile it back if compatibility with ES3 is not enabled.
 // This could be improved by inspecting Rollup's "generatedCode" option

packages/commonjs/src/index.js

@@ -16,6 +16,7 @@ import {
   EXTERNAL_SUFFIX,
   getHelpersModule,
   HELPERS_ID,
+  IS_WRAPPED_COMMONJS,
   isWrappedId,
   MODULE_SUFFIX,
   PROXY_SUFFIX,
@@ -115,16 +116,11 @@ export default function commonjs(options = {}) {
       return { meta: { commonjs: { isCommonJS: false } } };
     }
 
-    // TODO Lukas
-    // * test import from ESM -> additional proxy
-    // * test entry point
-    // * test interaction with dynamic require targets
-    // * test circular dependency: We must not use this.load without circularity check -> error in Rollup?
-    // When we write the imports, we already know that we are commonjs or mixed so we can rely on usesRequireWrapper and write that into a table
     const usesRequireWrapper =
       !isEsModule &&
       (dynamicRequireModuleSet.has(normalizePathSlashes(id)) || strictRequireSemanticFilter(id));
 
+    // TODO Lukas auto-detect circular dependencies
     // TODO Lukas extract helpers
     return transformCommonjs(
       this.parse,
@@ -181,8 +177,7 @@ export default function commonjs(options = {}) {
             return {
               source,
               id:
-                // TODO Lukas extract constant
-                isCommonJS === 'withRequireFunction'
+                isCommonJS === IS_WRAPPED_COMMONJS
                   ? resolved.id
                   : wrapId(resolved.id, PROXY_SUFFIX),
               isCommonJS

packages/commonjs/src/resolve-id.js

@@ -9,6 +9,7 @@ import {
   EXPORTS_SUFFIX,
   EXTERNAL_SUFFIX,
   HELPERS_ID,
+  IS_WRAPPED_COMMONJS,
   isWrappedId,
   MODULE_SUFFIX,
   PROXY_SUFFIX,
@@ -96,7 +97,7 @@ export default function getResolveId(extensions) {
       const {
         meta: { commonjs: commonjsMeta }
       } = await this.load(resolved);
-      if (commonjsMeta && commonjsMeta.isCommonJS === 'withRequireFunction') {
+      if (commonjsMeta && commonjsMeta.isCommonJS === IS_WRAPPED_COMMONJS) {
         return wrapId(resolved.id, ES_IMPORT_SUFFIX);
       }
     }

packages/commonjs/src/transform-commonjs.js

@@ -27,6 +27,7 @@ import {
   isRequireStatement,
   isStaticRequireStatement
 } from './generate-imports';
+import { IS_WRAPPED_COMMONJS } from './helpers.js';
 import { tryParse } from './parse';
 import { capitalize, deconflict, getName, getVirtualPathForDynamicRequirePath } from './utils';
 
@@ -521,9 +522,8 @@ function ${requireName} () {
     map: sourceMap ? magicString.generateMap() : null,
     syntheticNamedExports: isEsModule || usesRequireWrapper ? false : '__moduleExports',
     meta: {
-      // TODO Lukas extract constant
       commonjs: {
-        isCommonJS: !isEsModule && (usesRequireWrapper ? 'withRequireFunction' : true),
+        isCommonJS: !isEsModule && (usesRequireWrapper ? IS_WRAPPED_COMMONJS : true),
         isMixedModule: isEsModule
       }
     }

packages/commonjs/test/fixtures/form/defaultIsModuleExports-auto-reassign-exports-__esModule/output.js

@@ -2,8 +2,8 @@ import * as commonjsHelpers from "_commonjsHelpers.js";
 import { __module as inputModule, exports as input } from "\u0000fixtures/form/defaultIsModuleExports-auto-reassign-exports-__esModule/input.js?commonjs-module"
 
 (function (module) {
-module.exports = { __esModule: true, default: { foo: 'bar' }}
-}(inputModule));
+	module.exports = { __esModule: true, default: { foo: 'bar' }}
+} (inputModule));
 
 export default /*@__PURE__*/commonjsHelpers.getDefaultExportFromCjs(input);
 export { input as __moduleExports };

packages/commonjs/test/fixtures/form/defaultIsModuleExports-false-reassign-exports-__esModule/output.js

@@ -2,8 +2,8 @@ import * as commonjsHelpers from "_commonjsHelpers.js";
 import { __module as inputModule, exports as input } from "\u0000fixtures/form/defaultIsModuleExports-false-reassign-exports-__esModule/input.js?commonjs-module"
 
 (function (module) {
-module.exports = { __esModule: true, default: { foo: 'bar' }};
-}(inputModule));
+	module.exports = { __esModule: true, default: { foo: 'bar' }};
+} (inputModule));
 
 export default input.default;
 export { input as __moduleExports };

packages/commonjs/test/fixtures/form/defaultIsModuleExports-false-reassign-exports-no-__esModule/output.js

@@ -2,8 +2,8 @@ import * as commonjsHelpers from "_commonjsHelpers.js";
 import { __module as inputModule, exports as input } from "\u0000fixtures/form/defaultIsModuleExports-false-reassign-exports-no-__esModule/input.js?commonjs-module"
 
 (function (module) {
-module.exports = { default: { foo: 'bar' }};
-}(inputModule));
+	module.exports = { default: { foo: 'bar' }};
+} (inputModule));
 
 export default input.default;
 export { input as __moduleExports };

packages/commonjs/test/fixtures/form/typeof-module-exports/output.js

@@ -2,16 +2,16 @@ import * as commonjsHelpers from "_commonjsHelpers.js";
 import { __module as inputModule, exports as input } from "\u0000fixtures/form/typeof-module-exports/input.js?commonjs-module"
 
 (function (module, exports) {
-var foo = 42;
+	var foo = 42;
 
-if ( 'object' === 'object' && 'object' === 'object' ) {
-	module.exports = foo;
-} else if ( typeof undefined === 'function' && undefined.amd ) {
-	undefined([], function () { return foo; });
-} else {
-	window.foo = foo;
-}
-}(inputModule, input));
+	if ( 'object' === 'object' && 'object' === 'object' ) {
+		module.exports = foo;
+	} else if ( typeof undefined === 'function' && undefined.amd ) {
+		undefined([], function () { return foo; });
+	} else {
+		window.foo = foo;
+	}
+} (inputModule, input));
 
 export default input;
 export { input as __moduleExports };

packages/commonjs/test/fixtures/function/strict-require-semantic-circular/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+  description: 'handles circular dependencies with strict require semantic',
+  pluginOptions: {
+    strictRequireSemantic: true
+  }
+};

packages/commonjs/test/fixtures/function/strict-require-semantic-circular/main.js

@@ -0,0 +1,2 @@
+exports.foo = 'foo';
+t.is(require('./other.js').foo, 'foo');

packages/commonjs/test/fixtures/function/strict-require-semantic-circular/other.js

@@ -0,0 +1 @@
+exports.foo = require('./main.js').foo;

packages/commonjs/test/fixtures/function/strict-require-semantic-entry/_config.js

@@ -0,0 +1,20 @@
+const assert = require('assert');
+
+module.exports = {
+  description: 'strict require semantic modules can be entry points',
+  options: {
+    input: [
+      'fixtures/function/strict-require-semantic-entry/main.js',
+      'fixtures/function/strict-require-semantic-entry/other.js'
+    ],
+    output: {
+      chunkFileNames: 'generated-[name].js'
+    }
+  },
+  pluginOptions: {
+    strictRequireSemantic: ['fixtures/function/strict-require-semantic-entry/main.js']
+  },
+  exports(exports) {
+    assert.deepStrictEqual(exports, { foo: 'foo' });
+  }
+};

packages/commonjs/test/fixtures/function/strict-require-semantic-entry/main.js

@@ -0,0 +1 @@
+exports.foo = 'foo';

packages/commonjs/test/fixtures/function/strict-require-semantic-entry/other.js

@@ -0,0 +1 @@
+t.is(require('./main.js').foo, 'foo');

packages/commonjs/test/fixtures/function/strict-require-semantic-exportmode-exports/_config.js

@@ -1,5 +1,5 @@
 module.exports = {
-  description: 'supports using function wrappers for modules',
+  description: 'supports using function wrappers for modules for export mode "exports"',
   pluginOptions: {
     strictRequireSemantic: ['fixtures/function/strict-require-semantic-exportmode-exports/*E*.js']
   }

packages/commonjs/test/fixtures/function/strict-require-semantic-exportmode-exports/assignExports.js

@@ -1,2 +1,3 @@
 exports.foo = 'foo';
+module.exports.bar = 'bar';
 global.hasAssignExportsRun = true;

packages/commonjs/test/fixtures/function/strict-require-semantic-exportmode-exports/main.js

@@ -1,17 +1,17 @@
 t.is(global.hasAssignExportsRun, undefined, 'before require');
-t.is(require('./assignExports.js').foo, 'foo');
+t.deepEqual(require('./assignExports.js'), { foo: 'foo', bar: 'bar' });
 
 t.is(global.hasAssignExportsRun, true, 'after require');
 delete global.hasAssignExportsRun;
 
-t.is(global.hasReassignModuleExportsRun, undefined, 'before require');
-t.is(require('./reassignModuleExports.js').foo, 'foo');
-
-t.is(global.hasReassignModuleExportsRun, true, 'after require');
-delete global.hasReassignModuleExportsRun;
-
 t.is(global.hasCompiledEsmRun, undefined, 'before require');
-t.is(require('./compiledEsm.js').foo, 'foo');
+t.deepEqual(require('./compiledEsm.js'), { foo: 'foo', __esModule: true });
 
 t.is(global.hasCompiledEsmRun, true, 'after require');
 delete global.hasCompiledEsmRun;
+
+t.is(global.hasWrappedExportsRun, undefined, 'before require');
+t.deepEqual(require('./wrappedExports.js'), { foo: 'foo' });
+
+t.is(global.hasWrappedExportsRun, true, 'after require');
+delete global.hasWrappedExportsRun;

packages/commonjs/test/fixtures/function/strict-require-semantic-exportmode-exports/wrappedExports.js

@@ -0,0 +1,4 @@
+exports.foo = 'foo';
+exports = {};
+exports.bar = 'bar';
+global.hasWrappedExportsRun = true;

packages/commonjs/test/fixtures/function/strict-require-semantic-exportmode-module/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+  description: 'supports using function wrappers for modules for export mode "module"',
+  pluginOptions: {
+    strictRequireSemantic: ['fixtures/function/strict-require-semantic-exportmode-module/*E*.js']
+  }
+};

packages/commonjs/test/fixtures/function/strict-require-semantic-exportmode-module/assignModuleAndExports.js

@@ -0,0 +1,3 @@
+module.exports = { foo: 'foo' };
+module.exports.bar = 'bar';
+global.hasAssignModuleAndExportsRun = true;

packages/commonjs/test/fixtures/function/strict-require-semantic-exportmode-module/assignModuleExports.js

@@ -0,0 +1,6 @@
+if (Math.random() > 0.5) {
+  module.exports = { foo: 'foo' };
+} else {
+  module.exports = { foo: 'foo' };
+}
+global.hasAssignModuleExportsRun = true;