FredKSchott / rollup-plugin-polyfill-node

A modern Node.js polyfill for your Rollup bundle.
Other
176 stars 55 forks source link

Polyfill modules that have already been transformed by commonjs plugin #42

Closed ABuffSeagull closed 2 years ago

ABuffSeagull commented 2 years ago

So when trying to polyfill in modules with Vite, I kept running into problems that certain libraries were still not receiving the polyfills. I finally traced it down to the fact that these modules were already transformed by the commonjs plugin, and thus tried to import the builtins as commonjs with the name \0events?commonjs-<something> for example. Since it has already been "processed" by the commonjs plugin, this plugin would ignore it. So with this we can now tell the plugin to not ignore these modules and actually resolve them to the correct polyfill, but I'm not totally sure if we should be this direct. Should we also ignore other plugin-processed modules, or just the commonjs?

Also, to anyone else discovering this using Vite, you need to include this entire plugin under the normal plugins config option, not build.rollupOptions.plugins, as the latter will only use this plugin to resolve a single top level module, while the former will correctly use this plugin for every module resolution.

ABuffSeagull commented 2 years ago

Oh, and I think this would close #38 , #37 , #22 and probably others

FredKSchott commented 2 years ago

Interesting! nice find. A little hesitant to be messing with ids like this, but the use-case makes sense. I'll add a comment to point back to this PR for context.