search

rollup / rollup-plugin-commonjs

This module has moved and is now available at @rollup/plugin-commonjs / https://github.com/rollup/plugins/blob/master/packages/commonjs
MIT License
501 stars 126 forks source link

Can't specify namedExports of transitive dependencies using module names #419

Closed bterlson closed 4 years ago

bterlson commented 4 years ago

When setting the namedExports configuration object, you can specify paths or module names. Specifying module names is preferred when possible because it is not sensitive to layout of node_modules or presence of symlinks (on top of just being easier).

However, in the case of using a module name, it only works reliably for direct dependencies and not transitive dependencies because the plugin's initialization maps module names to resolved module paths by resolving from the project root. This is not guaranteed to resolve transitive dependencies depending on node_modules layout, especially when in a monorepo or using something like pnpm. I believe this also means if you have two versions of a library because of incompatible semver ranges, you won't be able to apply namedExports to both (and finding the exact path of either can be difficult and, I think, non-deterministic depending on install order and potentially other complexities).

The only way I can think to fix this right now is to NOT resolve module names to paths during initialization, and instead, when converting a CJS module grab its module name from package.json and compare with module names specified in the namedExports object.

Any other ideas?

lukastaegert commented 4 years ago

I agree with your assessment, though of course this is only correct in a Node scenario. Not sure how much work this would be but maybe the plugin could check for the new rollup-plugin-node-resolve api and if it is found, then use the new semantics that you outlined above, but if it is not found, then use the previous semantics (which appears to be more efficient to me). That way, there would be no hard coupling of this plugin to Node's import resolution, i.e. it could also be used in other CJS scenarios, but when Node is involved, then it would adapt automatically.

shellscape commented 4 years ago

Hey folks, we've moved this repo to a new home in our plugins monorepo at https://github.com/rollup/plugins. If this issue is still pertinent, please reopen, by filling out the appropriate issue template, in the new repo. Thanks for originally opening this issue, and we look forward to seeing you in the new repo. This issue is being closed and this repo is being permanently archived.