Support a promise paradigm

[rightaway]

Before

    yauzl.open('file.zip', (err, zipfile) => {
      if (err) throw err
      zipfile.on('entry', (entry) => {
        zipfile.openReadStream(entry, (err, readStream) => {
          if (err) throw err
          readStream.pipe(somewhere)
        })
      })
    })

After

    const zipfile = await yauzl.open('file.zip')
    zipfile.on('entry', async (entry) => {
      const readStream = zipfile.openReadStream(entry)
        readStream.pipe(somewhere)
      })
    })

If yauzl.open and zipfile.openReadStream return Promises if no callbacks are provided, it would simplify code for people who choose to use async/await. We could remove all if (err) throw err because unhandled errors would be thrown automatically. And we don't have as deep nesting of the code, which makes things hard to read unless functions are extracted which may otherwise not have needed to be extracted.

Of course, if callbacks are provided then no Promise is returned and code behaves as it currently does, making this change backwards-compatible.

Node v7, which will be released soon, will make native async/await available behind flags. And Node v8 will make them available without any flags. This feature would give the option to write simpler code for anyone using those versions of Node, or any version of Node with Babel (as I'm currently doing it).

[thejoshwolfe]

I think this is a great idea, but i don't think it's time to implement it yet. According to my research (and it seems hard to research this for some reason), async/await is not in any stable node version yet, so I'm not sure how i would deploy this change without breaking everything for stable node users.

[rightaway]

As I understand it nothing should break for existing users if promises are returned only when the callback isn't provided. So existing code would run as it currently does.

[thejoshwolfe]

Using the async keyword is a syntax error in old and current versions of v8.

for example: https://jsfiddle.net/0y7f0dkt/

How are you proposing I write JavaScript code in yauzl's index.js to return promises sometimes, and have that code not cause syntax errors in old and current versions of v8?

[rightaway]

As I understand it, yauzl's code itself wouldn't actually be using any async/await keywords. It would be returning a Promise when a callback isn't supplied, so that people who use yauzl in their code can use async/await.

An example is how ioredis package does it https://github.com/luin/ioredis/blob/b216e4eb61d05ff639b162f23751acb111dde6e8/lib/redis.js#L479-L510 for the https://github.com/luin/ioredis#monitor command.

Something like http://bluebirdjs.com/docs/api/ascallback.html or https://www.npmjs.com/package/nodeify could be used to make the yauzl API work with callbacks or promises.

[thejoshwolfe]

Oh. Promise is a builtin type in JavaScript since ES6. I'm less confused now. Sorry.

Currently, I don't believe yauzl depends on any ES6 features, and yauzl is supposed to work in very old versions of JavaScript and node. I'm not sure how valuable (or true) that is though, and maybe yauzl should embrace the new era of JavaScript. I'm generally resistant to ES6+ features, because I find them confusing to learn. I'm not just complaining about my own learning experience but also that of any potential contributor. Keeping yauzl's source easy to learn is a feature that I'm hesitant to give up, but I think I should probably be more open minded to the idea. (And if you think that the promise paradigm is easier to understand than the callback paradigm, then i guess we'll just have to disagree on that.)

There's still one thing left in this proposal that concerns me, which is that there would be multiple ways to use the library. I'm generally against programmer freedom, and instead value the pythonic one-obvious-way-to-do-it law of orthogonality design paradigm. I'm willing to compromise on this if there's a better way to do things, and I need to keep the old way around for compatibility (see #22). Then one way is deprecated, and the new way is the "obvious" way to do it. Are promises "better" than callbacks, or are they just different? They seem more syntactically convenient for some usecases, as you pointed out, but that's not a strong enough argument to convince me they're better.

I'm also concerned about the performance of promises vs callbacks. My hunch is that promises will perform slightly worse than callbacks, but I don't know. If promises have worse performance than callbacks, it's going to be pretty difficult to convince me that they're "better".

And after all that discussion, I'm not sure if I'm really being impartial on this issue, of if I'm letting my bias against ES6 to cloud my judgement. I'm open to discussion on all of these issues I've mentioned.

[andrewrk]

How does error handling work in promise land? In the code above, it's not obvious where the errors are. Thrown automatically in what context?

[rightaway]

I'm generally resistant to ES6+ features, because I find them confusing to learn. I'm not just complaining about my own learning experience but also that of any potential contributor.

More and more projects are adopting Promises (in addition to, not instead of, callbacks) in preparation for async/await support to be introduced in the upcoming v7 of Node, and to be available by default without a flag in v8 of Node coming out early next year.

So contributors to yauzl are probably already seeing Promise support in all kinds of projects that they're contributing to, and if not they will see it a lot more in the coming months. The only changes to yauzl code that contributors will see is the use of one of the helpers I linked to above in the few yauzl functions that return a callback. It would not be an invasive change to the yauzl codebase at all, and won't have any impact at all on users who would like to continue using callbacks. (No compatibility issues as it's not a breaking change, no performance issues, and yauzl will continue to work just fine in old versions of node and the browser.)

There's still one thing left in this proposal that concerns me, which is that there would be multiple ways to use the library. I'm generally against programmer freedom, and instead value the pythonic one-obvious-way-to-do-it law of orthogonality design paradigm.

This is a personal preference. On the other hand, many developers prefer consistency, and with yauzl getting millions of downloads a month on npm it would be useful if those developers could keep their code consistent, as opposed to being able to use async/await for most libraries apart from one or two that don't offer it as an option.

Are promises "better" than callbacks, or are they just different? They seem more syntactically convenient for some usecases, as you pointed out, but that's not a strong enough argument to convince me they're better.

The error handling benefits are a huge plus that is more than just cosmetic. When using callbacks, if you forget to check the error callback argument and rethrow it for example then that error will be missed completely and can cause tricky bugs. This isn't the case with async/await as the error gets thrown automatically.

I'm also concerned about the performance of promises vs callbacks. My hunch is that promises will perform slightly worse than callbacks, but I don't know. If promises have worse performance than callbacks, it's going to be pretty difficult to convince me that they're "better".

Native promises may have slightly worse performance at this time, but that will be improved as time goes on. It's still a new feature. People for whom performance is very important will do var Promise = require('bluebird') and get the advantages of bluebird's great performance. But anyway for a library like yauzl, promises will never be the performance bottleneck.

[martinkadlec0]

I think throwing the async/await terms around isn't really helping rightaway's case, nevertheless I would really appreciate if yauzl could work with promises as well, just so I could use 'then'. Using callbacks nowadays feels really unclean :)

[rightaway]

I think throwing the async/await terms around isn't really helping rightaway's case

I suspect you're right, do you mean it's better to talk about this in terms of promises rather than async/await?

[martinkadlec0]

Yes, I think wanting a Promise support just to get the Promises itself is completely valid.

[thejoshwolfe]

It came to my attention recently that "promisifying" API's that follow the node standard is pretty trivial. I added an example demonstrating how this can be done for yauzl with this simple promisifier (no bluebird or other third-party library required):

function promisify(api) {
  return function(...args) {
    return new Promise(function(resolve, reject) {
      api(...args, function(err, response) {
        if (err) return reject(err);
        resolve(response);
      });
    });
  };
}

(This implementation is very basic. It could certainly be improved with features like setting function names and stuff. Use at your own risk, etc.)

As a bonus, the example I added uses async and await to demonstrate that the original goal of this discussion thread is achievable with very little boilerplate.

[overlookmotel]

I have published an npm module yauzl-promise which promisifies the API.

Hope it's useful to someone.

[benmccann]

Thank you so much for yauzl-promise! It's so much friendlier than the underlying API!

Unzipping an archive with a single file is only a few lines of code!

const zipFile = await yauzl.fromBuffer(buffer);
const entry = await zipFile.readEntry();
const readStream = await entry.openReadStream();
const contents = await streamToString(readStream);
await zipFile.close();