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

Activity

1. lukastaegert commented on Nov 27, 2022

   

   lukastaegert

   on Nov 27, 2022

   Member

   More actions

   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.
2. lucaelin commented on Nov 27, 2022

   

   lucaelin

   on Nov 27, 2022

   Author

   More actions

   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.
3. lukastaegert commented on Nov 28, 2022

   

   lukastaegert

   on Nov 28, 2022

   Last edited by lukastaegert

   Member

   More actions

   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.
4. lukastaegert commented on Nov 28, 2022

   

   lukastaegert

   on Nov 28, 2022

   Member

   More actions

   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 } }
     };
   }
5. lucaelin commented on Dec 3, 2022

   

   lucaelin

   on Dec 3, 2022

   Author

   More actions

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

   closed this as completedin #1363on Dec 15, 2022