[@rollup/plugin-commonjs] does not provide an export named 'exports'

[lucaelin]

• Rollup Plugin Name: @rollup/plugin-commonjs
• Rollup Plugin Version: 22.0.2
• Rollup Version: 2.79.1
• Operating System (or Browser): Ubuntu, Chromium
• Node Version: v16.17.1
• Link to reproduction (⚠️ read below): https://github.com/lucaelin/rollup-commonjs-bug

I recently took a deep dive into some issues related to the @web/dev-server in conjunction with the commonjs plugin. You can read up on those related issues here. One remaining issue is with this plugin and the above error message. The commonjs-plugin injects an import statement using this logic here, but the generated output here does not provide the expected export-signature.

Apparently rollup itself does not show the same issue, which makes me wonder what I am missing in my analysis and if this is actually a bug in this plugin or if the @web/dev-server is actually at fault here. Any input is greatly appreciated!

Expected Behavior

web-dev-server should be able to transpile individual modules and pass module and import resolution over to the browser

Actual Behavior

The browser shows an error message, because the export signature of the ?commonjs-module stub does not match the signature of the generated import. Namely an export named exports is missing.

Additional Information

see above

[lukastaegert]

Sorry for the late reply.

The commonjs-plugin injects an import statement using this logic here, but the generated output here does not provide the expected export-signature.

You probably mean this line instead https://github.com/rollup/plugins/blob/master/packages/commonjs/src/index.js#L248 which is for MODULE_SUFFIX, not EXPORTS_SUFFIX. This one defines the __module export, but not the exports export. Instead it declares syntheticNamedExports: '__module', which means that if a file attempts a named export of something other than __module, then instead provide a property on the __module export. I.e. Rollup internally resolves this as __module.exports, and most importantly this is a live-binding that reacts to changes in __module, something you cannot as easily do with traditional ES exports. Unless we use proxies or accessors I guess. If you issues can be traced back to problems here, maybe this is something we should look into.

[lucaelin]

Oh, I see! Apparently I did slip with the lines I linked to. Thank you so much for the insight and yes, the issues so seem to be that browsers do not resolve the synthetic exports. I assume that the commonjs-plugin uses syntheticNamedExports here to allow mutation of the __module object to propagate to the exports export? Like

import { __module as ${moduleName}, exports as ${exportsName} } from '...';
const customExports = {a:1};
${moduleName}.exports = customExports; // replace the instance
${exportsName}.a = 2; // access the new instance

if this is the case then yes, it matches the issues seen in @web/dev-server.

[lukastaegert]

Yes, that is exactly what is happening and you gave a good example for what we try to handle with this. In that case, we might even get around it some other way. But we also use it to allow arbitrary named ESM imports from CommonJS, and here we cannot easily fake it. But if you have a solution for the other problem, or do not have this problem, we could fix the exports issue in some way.

[lukastaegert]

It appears that this approach would also satisfy all our test cases, but I will need to look a little deeper if there are potential issues:

if (isWrappedId(id, MODULE_SUFFIX)) {
  const name = getName(unwrapId(id, MODULE_SUFFIX));
  return {
    code: `var ${name}Exports = {};
           var ${name} = {
             get exports(){ return ${name}Exports; },
             set exports(v){ ${name}Exports = v; },
           };
          export {${name} as __module, ${name}Exports as exports}`,
    meta: { commonjs: { isCommonJS: false } }
  };
}

[lucaelin]

I ran your code against some of the problematic modules we discovered on @web/dev-server and it seems to be working great!