search

kriskowal / zip

An implementation of unzip in JavaScript for Node
MIT License
85 stars 28 forks source link

Why not using the Nodejs zlib module to do inflate #7

Open leric opened 11 years ago

leric commented 11 years ago

The most compuational expensive part of unzip is the inflate operation, why not use nodejs bundled zlib module, which based on zlib C implementation, that might be more simpler and faster

kriskowal commented 11 years ago

It’s a good idea and I’ll consider it. I would like to retain the pure-js Inflate implementation so we can eventually make this package work both server and client side.

kriskowal commented 11 years ago

I’ve decided that I’ll land a pull request if it comes in. The current inflate should be moved into a browser directory and be replaced by an adaptor for zlib that has the same interface. The interface of inflate and browser/inflate may be changed as long as it’s consistent.

leric commented 11 years ago

There a problem to use zlib: it's async, I don't know if there's a way to use a async lib while remain the current sync API. It might need to create a new API to use zlib.inflate

deathcap commented 10 years ago

I would like to retain the pure-js Inflate implementation so we can eventually make this package work both server and client side.

For what it's worth, in browserify the zlib module is automatically replaced with the browser-compatible https://github.com/brianloveswords/zlib-browserify - no code changes are necessary for both browser/NodeJS support

However, zlib-browserify uses a shimmed Buffer instance, which can cause problems, per https://github.com/chrisdickinson/bops/pull/7

Ah, the problem is that zlib.gunzip is a browserify shim, and it returns browserify Buffer shim instances. bops is expecting a Uint8Array to be passed around while in the browser, and that's why it explodes with a TypeError.

Since zip is using its own inflate implementation instead, it was fairly easy to port it to use bops in https://github.com/kriskowal/zip/pull/13 - which wouldn't necessarily be the case if zip switched to zlib (maybe a bops-compatible zlib would be best bundled in browserify? @substack), though having a separate browser zlib could be a solution.