-
Notifications
You must be signed in to change notification settings - Fork 232
Remove requirement for bundler custom config #489
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR! This change would need to be applied to our build/ scripts, not inside lib.
@ShogunPanda could you review? |
LGTM, if you could address #489 (comment) it would be great. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The current code requires adding custom configs for bundlers to import Node.js polyfills. This PR removes that requirement.
Polyfills are required directly in the code. This can be done because the Node.js core modules have the same names (
buffer
,events
,process
), and therefore, the code modules are used under Node.js. The polyfills are only imported for browser builds.This required switching from
buffer-es6
tobuffer
, andprocess-es6
toprocess
. Both packages are more up-to-date than the ones that have been replaced.Many lines have been touched, but the
lib
folder changes are pretty small. Most changes are related to the test code. The same imports are required, but more files are affected.Besides the globals
buffer
andprocess
,events
is added as a dependency. No code changes are required in that regard becauseevents
is not accessible as global, and therefore,require
calls are already in the current code.