Updated and more complete version of the zlib shim

[dignifiedquire]

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

[daviddias]

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.

[haadcode]

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.

[SpaceK33z]

@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.

[dignifiedquire]

@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.

[daviddias]

@dignifiedquire go ahead and publish it to npm, name suggestion browserify-zlib-complete :)

[SpaceK33z]

@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.

[dignifiedquire]

Published https://www.npmjs.com/package/browserify-zlib-next and created PR #52

[daviddias]

beep boop. Can we get an update from the devs on this? Thank you!

[wires]

Bump ;)
Ran into this, would be nice if it can get merged and released cheers 👍

[dignifiedquire]

This is fixed in browserify-zlib@0.2.0: http://npmjs.com/browserify-zlib