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

Export missing globals for compat #347

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Conversation

chjj
Copy link
Contributor

@chjj chjj commented Feb 3, 2024

No description provided.

@dcousens
Copy link
Collaborator

dcousens commented Feb 5, 2024

Buffer doesn't export these in node? What's the intent?

@chjj
Copy link
Contributor Author

chjj commented Feb 5, 2024

$ node
Welcome to Node.js v21.6.1.
Type ".help" for more information.
> require('buffer').Blob
[class Blob]
> require('buffer').File
[class File extends Blob]
> require('buffer').atob
[Function: atob]
> require('buffer').btoa
[Function: btoa]

@dcousens

This comment was marked as resolved.

@dcousens dcousens merged commit d0092f7 into feross:master Feb 5, 2024
6 checks passed
@dcousens
Copy link
Collaborator

dcousens commented Feb 5, 2024

Thanks @chjj 🧡

@chjj
Copy link
Contributor Author

chjj commented Feb 5, 2024

This is silly, but I'm now actually having second thoughts on this one.

I know this is compatible with browserify as browserify exposes global to the module, but I'm curious about other bundlers: parcel/webpack/rollup/esbuild/etc (I know the latter two steer their users towards ES modules, but I think they have some kind of CJS compatibility plugins too?).

Maybe we should switch this to a typeof x !== 'undefined' pattern to avoid potentially breaking non-browserify bundlers?

Or maybe I'm just being paranoid and they all support global.

@dcousens
Copy link
Collaborator

dcousens commented Feb 5, 2024

I'm happy with the cautious approach (typeof x !== 'undefined'), but ideally, can we test this package easily in our CI with some other bundlers?

chjj added a commit to chjj/buffer that referenced this pull request Mar 1, 2024
@chjj chjj mentioned this pull request Mar 1, 2024
dcousens pushed a commit that referenced this pull request Mar 1, 2024
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.

2 participants