Updated and more complete version of the zlib shim
dignifiedquire opened this issue · 10 comments
There is a new version of the browserify-zlib shim to zlib available at https://github.com/ipfs/browserify-zlib, it fixes some bugs and more importantly, it adds support for dictionaries.
It seems that not a lot of people use that feature, but it is essential for libp2p-spdy in the browser, which is part of js-libp2p and js-ipfs. Right now we are replacing the shim in our build tool, but it would improve the experience a lot if everyone using it could just rely on webpack to use the correct version of the shim.
We've a open PR -- browserify/browserify-zlib#18 -- to the bundled browserify-zlib in browserify, but 8 months have passed since it was open, the author and maintainer seems to be lacking the time to review. What else an we do to move this forward?
Thank you :)
There is also an issue open on browserify for this: browserify/browserify#1672 which suffers from the same problem.
cc @diasdavid
The number of bug reports that modules are broken with a zlib error keeps increasing and simply because we can't force WebPack builds to use ipfs/browserify-zlib in the same we do to replace other modules that are loaded into the browser, it is super painful to everyone.
Merging this would be hugely beneficial for the https://github.com/ipfs/js-ipfs project.
We're currently working towards supporting https://github.com/facebookincubator/create-react-app which doesn't support custom webpack configs. As js-ipfs in itself uses a fork of browserify-zlib, we need to include it in our build tools (https://github.com/dignifiedquire/aegir/blob/master/config/webpack.js#L35). As such, we're not able to have create-react-app compile code that uses js-ipfs, because we can't include the fork :/ I hope this makes sense, but let me know if clarification is needed.
If there's anything I (and we) can do to get closer to merging this, please let us know.
@haadcode for us it's important to know if there are breaking changes in the new library compared to the curret one and if so, which one. Also a PR implementing this would be appreciated.
@SpaceK33z thank you for the response. There should not be any breaking changes, only bug fixes and feature additions. I looked through the changelogs again and there where no breaking changes listed for zlib in v4, v5 or v6
If there is interest in shipping this I would publish the fork to npm on a new name and open a PR here. That change would only consist in changing the name of the zlib dependency and its version.
@dignifiedquire go ahead and publish it to npm, name suggestion browserify-zlib-complete :)
@dignifiedquire okay, without breaking changes we can publish this as a minor version. Yes publishing it on npm and opening a PR would be a good idea. Note that I don't maintain this repo myself, but I'll try to get some others on it after your PR.
Published https://www.npmjs.com/package/browserify-zlib-next and created PR #52
beep boop. Can we get an update from the devs on this? Thank you!
Bump ;)
Ran into this, would be nice if it can get merged and released cheers 👍
This is fixed in browserify-zlib@0.2.0: http://npmjs.com/browserify-zlib