utils: add makeNodeResolver function. #488
Conversation
Many Firebase methods take Node.js style callbacks where the first argument is null do indicate success or contains error information. Angular uses promises to handle asyncronous flow. The original [Q](https://github.com/kriskowal/q) library by kriskowal (upon which had Angular promises are based) had a `makeNodeResolver` function that eased integration of the two different async flows. This utility function replicates that feature. Usage is as follows: ```javascript var defer = $q.defer(); ref.resetPassword( {email:'somebody@somewhere.com'}, $util.makeNodeResolver(defer) //automatically resolves/rejects promise ); defer.promise.then(...) // use the promise as you normally would ```
|
Changes Unknown when pulling d6f09ba on jamestalmage:utils-node-resolver into * on firebase:master*. |
|
a lot of angularFire functions can benefit from this (almost everything in FirebaseAuth will) |
|
Changes Unknown when pulling afc3fe7 on jamestalmage:utils-node-resolver into * on firebase:master*. |
|
Seems FirebaseAuth is the only place you are using promises, so those are the only internal methods that benefit. I say it still belongs on the utility object, but I can move it to FirebaseAuth if you want. |
|
I did an offline merge and the refactoring still passes the new FirebaseAuth test suite. |
| expect(deferred.reject).toHaveBeenCalledWith(error); | ||
| }); | ||
|
|
||
| it('should resolve the promise if the first argument is falsy', function(){ |
There was a problem hiding this comment.
I think I would like to see these methods specifically check for null. Node, and the Firebase methods, both pass null for the error if it is a success, so anything else would be fishy and either a bug or error.
There was a problem hiding this comment.
OK, makes sense. They don't do that check in the original library, but according to the Node.js spec, error should be null. I've made the change
| var result2 = {data:'howdy!'}; | ||
| callback(null,result1,result2); | ||
| expect(deferred.resolve).toHaveBeenCalledWith([result1,result2]); | ||
| }); |
There was a problem hiding this comment.
I just added this as well. ^
Conflicts: src/FirebaseAuth.js
utils: add makeNodeResolver function.
|
-0.04%!! There goes the farm. |
|
This is super useful and great addition. 👍 |
Many Firebase methods take Node.js style callbacks where the first
argument is null do indicate success or contains error information.
Angular uses promises to handle asyncronous flow. The original
Q library by kriskowal (upon which Angular promises are based)
had a
makeNodeResolverfunction that eased integration of the twodifferent async flows.
This utility function replicates that feature. Usage is as follows: