feat($q): add streamlined ES6-style interface for using $q #8311
Conversation
|
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
|
@jeffbcross I've stuck this chunk of the $q CL here to be reviewed separately, in case step 2 isn't ready tomorrow. Can you take a quick review at it? It does still contain the duplicated set of tests, which are irrelevant for this CL, but if the next one lands, it will be very relevant, so I think it would be good to keep them. I'll remove them if that's too much of a problem though. |
|
|
||
| $Q.defer = defer; | ||
| $Q.reject = reject; | ||
| $Q.when = when; |
There was a problem hiding this comment.
Shouldn't this be $Q.resolve?
There was a problem hiding this comment.
Oh, wait, I see. You're using the same constructor to power the new and old $q
There was a problem hiding this comment.
I at first thought this was intended to match semantics of ES6 Promise.reject and Promise.resolve
There was a problem hiding this comment.
I don't see a problem with adding a resolve() or resolved() helper too, but I'm trying to keep this one as simple as possible
| * $q can be used in two fashions --- One, which is more similar to Kris Kowal's Q or jQuery's Deferred | ||
| * implementations, as well as a new ES6-inspired usage. | ||
| * | ||
| * The streamlined ES6 style promise can be used like so: |
There was a problem hiding this comment.
Should be a little more specific about the extent of ES6-ness. I.e. the constructor+resolver function is the only thing brought in.
|
Other than one doc nitpick, LGTM |
This potentially helps lead the way towards a more performant fly-weight implementation, as discussed earlier in the year. Using a constructor means we can put things in the prototype chain, and essentially treat $q as a Promise class, and reuse methods as appropriate. Short of that, I feel this style is slightly more convenient and streamlined, compared with the older API.
|
Okay, the docs have been reworked: http://ci.angularjs.org/job/angular.js-caitlin/426/artifact/build/docs/api/ng/service/$q If that looks good, I think it can land. I do think that the docs for this section might be re-organized further, but that's a separate issue. |
|
I like the docs change, LGTM to merge |
|
okay, will do |
|
curious to see a benchmark... constructor vs property |
|
We don't have an exact benchmark of refactored $q vs old $q (because making that work seems hard) --- but there is this: http://jsperf.com/angular-q-vs-bluebird2/2 (refactored) vs http://jsperf.com/angular-q-vs-bluebird2 (not refactored) --- note the difference |
|
Thanks for this, looking forward to future promise improvements. |
This potentially helps lead the way towards a more performant fly-weight implementation, as discussed earlier in the year. Using a constructor means we can put things in the prototype chain, and essentially treat $q as a Promise class, and reuse methods as appropriate.
Short of that, I feel this style is slightly more convenient and streamlined, compared with the older API.
(#8067 originally contained this PR, but has been refactored into a staging branch for the remaining changes left in #8300 --- This should be much easier to review as the first of a set of 3 changes).