Upgrade to the latest #18
Conversation
Latest pako & dictionary support
Just rewrote binding.js, was easier :)
Update karma-chrome-launcher to version 1.0.1 🚀
Update karma-mocha to version 1.0.1 🚀
|
@dignifiedquire great work! Much better than #17! |
|
@fanatid #17 was before we knew that pako was missing the dictionary implementation :) @dignifiedquire did a great job at getting everything done, well tested and did a revamp on browserify-zlib, which we are now using for libp2p+ipfs. |
|
@diasdavid ha, didn't notice that you are also from Protocol Labs :) |
|
@fanatid not at the moment, there is not much, but some changes have been made from |
Update karma to version 1.1.0 🚀
|
This PR fixes #13, #14, #15, #20, and the |
Update mocha to version 3.0.1 🚀
|
@jscissr is there anything else you would like to see in this PR for the merge? |
|
No, for me everything seems to work fine. |
|
what about you, @fanatid , looks good to merge? :) |
|
@diasdavid I can't review all this code, but if this new code compliant with node module (at least with 5.x) and passes all tests from node -- it's good for me. |
|
I like the idea of this pull: the module has gotten kinda old. But I think as much of the code as possible should be a direct copy paste from the node source code, including the tests, as I did originally. It will be much easier to maintain: we can just copy paste again in the future. Thoughts? |
Update standard to version 8.0.0 🚀
Update README URLs based on HTTP redirects
|
ping @devongovett |
|
🙏 |
|
@devongovett any chance of this happening? I am also happy to help maintain this module if you don't have the time/ don't want to. But releasing this would benefit the whole browserify/webpack community, who all use this module for zlib in the browser. |
|
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. |
In the browser different versions of the `Buffer` package can be around, making it impossible to rely on `instanceof` checks.
|
Hey sorry guys. I realized I'm completely failing at maintaining this package. @dignifiedquire I've added you as a collaborator on this repo. If you tell me your npm username, I'll add you there too so you can publish. I think this PR looks pretty good with the modifications you made to use the original code from node. As I mentioned previously, my main goals for this repo originally were to use as much code from node as possible, and to run the tests without modifications as well. This ensured that the browserify version worked in exactly the same way as node. Would be happy if these goals were followed going forward, but I don't have time to personally maintain it anymore. Thanks in advance! |
|
Thank you @devongovett my npm username is the same |
|
Awesome, you have been added. |
|
Thank you, I will prepare the PR to be ready for merge tomorrow and hopefully push out a release then :) |
|
Thanks for doing this @dignifiedquire. Can you also make a PR to browserify and webpack to update their dependencies? I don't think they will pull in the new version automatically because of the way semver works. https://github.com/substack/node-browserify/blob/master/package.json#L27 |
It's unclear where the "major" version bump came from, they do have a number of changes, but they seem mostly to be changing test framework, upgrading dependencies (I care about the transitive `pako` upgrade), and the like. The project is kinda quiet. Note that the 1.0 release is of a different, temporary name, while the PR below was trying to merge. It merged in 2017, and they released it, as 0.2, instead of as 1.0. https://github.com/browserify/browserify-zlib/commits/master browserify/browserify-zlib#18
This PR does a couple of things that were much needed for us
5.9.15.9.1to mocha as we did not want to import the custom test runner node-core is usingThis is a very big change and I understand if you have concerns about this. As @diasdavid already mentioned in #16 we will be needing this module in the forseeable future and maintaining it. Hopefully we can find a solution for that without having to maintain all this in a fork.