Skip to content

Update deps, add readable-stream #14

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

Conversation

rvagg
Copy link
Contributor

@rvagg rvagg commented May 1, 2020

the main thing here is switching out stream for readable-stream. With Webpack not updating their node-libs-browser we're not getting a newer version of stream-browserify so things break because of breaking changes in readable-stream. So this just bypasses that problem entirely and goes straight to readable-stream which it's using anyway. Also pinning to readable-stream gives some nice stability guarantees.

I'm using this through ripemd160 for btc stuff btw, 👍 on that.

@fanatid
Copy link
Collaborator

fanatid commented May 1, 2020

Thanks. Am I understand correctly, that this can be published as minor version?

@rvagg
Copy link
Contributor Author

rvagg commented May 1, 2020

Yep, very safe, readable-stream is the source of the Node core stream package, development happens there and it moves into core. So you get the same version as in the latest Node but you also get it in whatever version of Node you're using when consumed like this. readable-stream is tested back to Node version 6 still too.

@fanatid fanatid merged commit 6e8aa98 into browserify:master May 1, 2020
@fanatid
Copy link
Collaborator

fanatid commented May 1, 2020

Published as 3.1.0

@danimoh
Copy link

danimoh commented Oct 20, 2020

Hello @rvagg, I'd be interested in what the breaking changes are that you were talking about and how they affect this library or ripemd160?
I'm currently using this library in a rollup project but encountering problems with circular dependencies.
When using an older version of readable-stream (to be exact when using stream which is then polyfilled for the browser by an older readable-stream version via rollup-plugin-node-polyfills) I'm encountering at least less circular dependencies and can generate code that runs.

@rvagg rvagg deleted the rvagg/update-deps-add-readable-stream branch October 21, 2020 02:43
@rvagg
Copy link
Contributor Author

rvagg commented Oct 21, 2020

nodejs/readable-stream@c5897e3 this

The removal of the top-level entry points is the main one i keep on hitting. I can't recall if it was coming up here, but probably. Some folks got in the habit of writing things like require('readable-stream/duplex') (or just 'stream/duplex' which worked in the browser at least). I wish those files were just left alone because it's been so disruptive and the Webpack decision to lock readable-stream to v2 has made it harder.

@danimoh
Copy link

danimoh commented Oct 21, 2020

Alright, thanks for the answer!
I guess in my case I'm then safe for now with still using stream instead of readable-stream.
Hopefully they get the circular dependencies in readable-stream sorted out soon.

@ljharb
Copy link
Member

ljharb commented Mar 5, 2024

turns out this was a breaking change in that it drops node < 4, which v3.0.0 supports.

ljharb added a commit to browserify/crypto-browserify that referenced this pull request Mar 5, 2024
ljharb added a commit to browserify/browserify-sign that referenced this pull request Mar 5, 2024
ljharb added a commit to browserify/parse-asn1 that referenced this pull request Mar 5, 2024
ljharb added a commit to browserify/parse-asn1 that referenced this pull request Mar 5, 2024
ljharb added a commit to browserify/createHash that referenced this pull request Nov 15, 2024
ljharb added a commit to browserify/createHash that referenced this pull request Nov 15, 2024
ljharb added a commit to browserify/createHash that referenced this pull request Nov 15, 2024
@ChALkeR
Copy link
Contributor

ChALkeR commented Nov 16, 2024

@ljharb this was done in #11 before this PR

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.

5 participants