ericelliott / credential

Easy password hashing and verification in Node. Protects against brute force, rainbow tables, and timing attacks.
MIT License
347 stars 28 forks source link

callback, promise or both #44

Closed mastilver closed 8 years ago

mastilver commented 8 years ago

currently the api is using callbacks What about using Promises instead?

Or we can have both (return Promise if callback is not provided) I'm sure there is a module that does that, I can't remember witch one...

What do you think?

tjconcept commented 8 years ago

:+1: This is exactly what I did with credentials using Bluebird (see it's nodeify or asCallback method).

mastilver commented 8 years ago

nodeify was the one I was looking for :+1: I'm not a big fan of bluebird (mainly because it's not following the standard) I prefer pinkie

tjconcept commented 8 years ago

Where does it violate the standard?

brianmhunt commented 8 years ago

Bluebird is Promises/A+ v1.1 compliant, and bluebirdjs.com notes:

Spec compatible — bluebird can work as a drop-in replacement for native promises for an instant performance boost. It passes the Promises/A+ test suite and is fully spec compliant.

@mastilver, do you just mean bluebird has a bunch added on top of the standard? (as in pinkie - minimal spec-compliant polyfill)

mastilver commented 8 years ago

@brianmhunt yeah, this is what I mean

By using pinkie, I'm sure that when I switch to es6, I don't have to rewrite any code. I will just need to drop require('pinkie')

brianmhunt commented 8 years ago

@mastilver Got it; yeah, I agree, that's a problem. IMHO it is imprudent for libraries to use Promise facilities outside ES6.

ericelliott commented 8 years ago

There are lots of utilities that can convert a callback API to a promise API, but I would consider a minimal addition that returns a minimal ES6 promise if a callback is not provided.

I'd prefer a minimal standards-based polyfill for the promise lib.

mastilver commented 8 years ago

Cool, I will try to create a PR tomorrow

ericelliott commented 8 years ago

=)

joepie91 commented 8 years ago

The point of Promises/A+ is that implementations can add whatever functionality they consider necessary, as long as base interoperability is accomplished (which it is). There is no problem whatsoever with Bluebird providing additional methods, nor are ES6 Promises a good baseline - they're just yet another A+ implementation (and one that is severely lacking in several areas, such as error handling and debuggability).