Skip to content

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

Merged
merged 3 commits into from
Sep 20, 2022
Merged

Conversation

bergos
Copy link
Contributor

@bergos bergos commented Sep 14, 2022

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 to buffer, and process-es6 to process. 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 and process, events is added as a dependency. No code changes are required in that regard because events is not accessible as global, and therefore, require calls are already in the current code.

Copy link
Member

@mcollina mcollina left a 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.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@mcollina
Copy link
Member

@ShogunPanda could you review?

@bergos bergos requested a review from ShogunPanda September 19, 2022 08:01
@ShogunPanda
Copy link
Contributor

LGTM, if you could address #489 (comment) it would be great.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@bergos bergos requested a review from mcollina September 20, 2022 09:07
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

3 participants