-
Notifications
You must be signed in to change notification settings - Fork 1.2k
builtins - provide streams3 via readable-stream@3 #1938
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
|
Does this affect which engines a Browserify bundle can run on, or only which engines Browserify itself runs on? |
|
I believe bundles using readable-stream@2 already worked on IE 11 and up because of the |
|
Yeah I believe this involves dropping node versions <4 and perhaps some browsers as well. Slap it behind a major version number and lets let the past be the past 🚀 |
|
Please don’t drop older browsers. That choice is for app devs, not for a bundler to make. |
|
@ljharb disagree! this is what semver is for. app devs retain the option to build with older versions of browserify. Currently browserify is failing in its mission to provide the node environment (subset) in a browser bundle by not shipping streams3 |
|
@ljharb @goto-bus-stop Errors in CI over at #1937 seem to imply that browserify's deps no longer support |
|
What semver is for is to communicate breakage; that in no way means we have to capriciously break people. can you explain more about what is achieved by using streams3 that can’t be done with the current impl? |
|
If deps have stopped supporting node versions without a major bump, we should peg them and file upstream bugs. |
just one example of api not matching latest of node v10 readable-stream on v2-no-util-mut [?] is 📦 v2.3.6 via ⬢ v10.17.0
➜ echo 'console.log(require("stream").pipeline)' | node
[Function: pipeline]
readable-stream on v2-no-util-mut [?] is 📦 v2.3.6 via ⬢ v10.17.0
➜ echo 'console.log(require("stream").pipeline)' | browserify - | node
undefinedsome behavioral differences documented here https://github.com/nodejs/readable-stream#version-3xx
I agree, just wanted to point this out |
|
Is there any way to keep both and conditionally require it, or a simple way for browserify users to fall back to the older impl? |
|
you can pass in |
I agree that there's no reason to break things for no reason, but supporting v0.8 will continue to be a barrier for changes going forward with though I suppose there can be a distinction in platform support for the bundle we output vs the node version we run on |
|
@goto-bus-stop @ljharb anyways, since there is going to be significant resistance to dropping support, would you mind adding a +1 to this |
|
some scribbles #1940 |
This is a breaking change (see https://github.com/nodejs/readable-stream#version-3xx)
Fixes #1880