Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add node-polyfills #92

Closed
wants to merge 1 commit into from
Closed

Add node-polyfills #92

wants to merge 1 commit into from

Conversation

curran
Copy link
Contributor

@curran curran commented Jul 24, 2021

Awesome Contribution Checklist:

  • I have read, and re-read the Contributing Guidelines
  • I have searched to ensure the suggested item doesn't exist on this list
  • This PR contains only one item

Please Provide a Link A Repository for Your Addition

https://github.com/ionic-team/rollup-plugin-node-polyfills

Please Describe Your Addition

This is a mature package that polyfills Node builtins (such as EventEmitter). It is actually recommended by a Rollup warning that reads as follows:

Creating a browser bundle that depends on Node.js built-in modules ("events"). You might need to include https://github.com/ionic-team/rollup-plugin-node-polyfills

So, I was surpised that it's not on this list! As I've been using this list as a reference so frequently, I figured I'd open this PR to add this entry.

Thanks for maintaining!

@shellscape
Copy link
Contributor

Unfortunately that package is no longer maintained and the maintainers are next to impossible to work with. The rollup project no longer recommends it.

@curran
Copy link
Contributor Author

curran commented Jul 24, 2021

Oh I see. Thanks for the tip!

Out of curiosity, what is the recommended way to get Node polyfills in a Rollup context? Browserify plugin?

@curran curran closed this Jul 24, 2021
@curran
Copy link
Contributor Author

curran commented Jul 24, 2021

@shellscape
Copy link
Contributor

yeah there have been some open issues about that line of code. while that legacy bit of code stays, we can't in good conscience add that to the list here for the reasons stated. there have been some other projects attempting to fill the void, but everything ends up incomplete and it's a bit overwhelming for maintainers to tackle

@curran
Copy link
Contributor Author

curran commented Jul 24, 2021

Understood. Thanks!

I think I'll give the Browserify Rollup plugin a spin and see if that does the job. I recall from years ago that Browserify did polyfill Node builtins, but haven't touched it in years.

@curran
Copy link
Contributor Author

curran commented Jul 24, 2021

Oh wow, that https://www.npmjs.com/package/rollup-plugin-browserify-transform module hasn't been published in 4 years.

@curran
Copy link
Contributor Author

curran commented Jul 24, 2021

Found that other issue: Stop recommending node-globals/node-builtins plugin #2881.

@curran
Copy link
Contributor Author

curran commented Jul 24, 2021

Submitted a patch upstream rollup/rollup#4190

Looks like https://github.com/snowpackjs/rollup-plugin-polyfill-node is the clear winner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants