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

chore(deps): remove unnecessary dependency on buffer #139

Closed
wants to merge 1 commit into from

Conversation

ObserverOfTime
Copy link

Node.js provides a built-in buffer module which is what this package actually uses there.
The buffer package is meant to be used in the browser which, as far as I can tell, is not supported.

@rvagg
Copy link
Owner

rvagg commented Mar 11, 2024

The buffer package is meant to be used in the browser which, as far as I can tell, is not supported.

That's what it's here for, I believe that there are still users that need this dependency for it to work nicely in the browser; at least browserify used to require it. I know there are ways to hack it with Webpack, although even there it now prefers explicit inclusion.

When running in Node.js, this package won't get used, it's just cruft in your dependency tree, which I guess is what you're taking issue with. But it's there for browser users. Until I hear otherwise from someone who cares more about browser bundling, I'm going to leave it here.

@rvagg rvagg closed this Mar 11, 2024
@ObserverOfTime
Copy link
Author

ObserverOfTime commented Mar 11, 2024

Your readme does not mention browser support.

A Node.js Buffer list collector, reader and streamer thingy.

@ObserverOfTime
Copy link
Author

ObserverOfTime commented Mar 11, 2024

I know there are ways to hack it with Webpack, although even there it now prefers explicit inclusion.

Any project that needs this package in the browser should be the one to also depend on buffer and use a fallback.
There are similar solutions for Rollup/Vite and browserify appears to include it by default.

This package, which is primarily designed for Node.js, should not depend on something it doesn't explicitly need.

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