-
Notifications
You must be signed in to change notification settings - Fork 27.4k
$q is probably due for a rewrite #5223
Comments
Can you provide a link to the 2.x suite of tests and perhaps the result of running $q against it? |
Its all at https://github.com/promises-aplus/promises-tests. The easiest Once you do that, run the tests using grunt, but before you do, be sure to Before you change the adapter, every test will fail. After you change the (1) callbacks must be called with the global object in loosey-goosey mode To that end, efficiency demands that you handle native promises separately, The current $q is a bit of a mess with an eye toward those changes. I am For obvious reasons, I am retaining so much of the existing code as I can, Once I have the refactoring done to satisfy the existing test suite, I will Current footprint is more than 750 bytes of stack per promise, and should On Fri, Dec 6, 2013 at 2:28 PM, Pete Bacon Darwin
|
Okay, so I'm going to take this on I guess! First things first, getting tests passing the v2.0.3 suite. So far, 323 are passing, and 500 are failing. Some of these seem pretty trivial to fix, so I'll try to get those working, and then we can look at putting all the proper functionality into a prototype. This could take me a while, since there are a lot of failures :( @wizardwerdna if you'd like to help I'm happy to collaborate --- The big block of failures is in the 2.3.3 suite of tests :( |
I can pass along some guidance as to key areas to give attention. In the
If you nail these four issues, I would be surprised if you aren't finished. If you would like another set of eyes, please advise. I'll sit back on the On Mon, Dec 16, 2013 at 12:44 PM, Caitlin Potter
|
Alright how about we do this, I'll push my current WIP stuff to github, and we can hack on it to try and get the remaining tests passing. Currently I'm only running the 2.3.3 suite because it is the biggest source of problems, and I've got about 30 of them passing (at minimum, since I've xdescribe'd most of the suites so that they're easier to manage) -- But my changes to the test suite itself obviously won't show up on github, heh. Give me a few minutes and I'll push that stuff up if you'd like to take a look. For sure any changes that I make to $q will need to go through some serious review before it's landable https://github.com/caitp/angular.js/tree/aplus203 << my work-in-progress hmm, it might actually be easier to start totally from scratch on v2.0.3 stuff, but I'll take another look tomorrow. |
Will do. I tried that approach iniitally, and decided it needed a serious On Mon, Dec 16, 2013 at 4:19 PM, Caitlin Potter notifications@github.comwrote:
|
Unsure if this is relevant or right place for this point but native promises are coming to JS - and with a different API than $q. Refer to the native Promise implementation here: http://www.html5rocks.com/en/tutorials/es6/promises/. Would it be appropriate to adopt the native API to be more future proof? |
We've talked about that a bit today @hswolff, but I guess it's probably going to be a while before we can really delegate things to the native es6 implementations (although definitely, that would be awesome) |
Its a fine place to begin the discussion. Yes, native promises are coming, function defer(){
var deferrable;
new Promise(function(resolve, reject, progress){
deferrable = {
promise: this,
resolve: resolve.bind(this),
reject: reject.bind(this),
progress: progress.bind(this)
}
}
return deferrable;
} You can likewise produce an adapter for promise objects, and you should be On Mon, Dec 16, 2013 at 7:03 PM, Caitlin Potter notifications@github.comwrote:
|
Related to native promises: Wondering if Angular would ever support a similar type of API: var promise = $q.promise(function(resolve, reject) {
//...
}); |
@ewins I'm a fan of the resolver function promise API. |
@wizardwerdna that PR is passing the aplus tests, but unfortunately it breaks some of the extra API, and it looks like using a strategy similar to Q to unbreak it would add a significant amount of code :( when you get some time it would be great if you could poke through it and see what it could do better |
I will do. Find it odd that we are in this situation -- my experience is Best. On Thu, Dec 26, 2013 at 11:35 AM, Caitlin Potter
|
yeah, it's a bit weird. It only seems to break the |
Any update on this since January? |
@DavidSouther @lgalfaso has a PR for updating $q which we need to land, that will probably not happen this week, but we want to get that in before shipping 1.3 |
Last time |
@rayshan that's not going to happen in 1.x. Work is already being done for this in angular 2.0 (https://github.com/jeffbcross/deferred was related to this, for example) |
I see, that works, thanks for the update @caitp. |
Closing as most of the original issues have since been addressed. |
Given the importance of promises to the angularJS way, it is probably worth spending some time considering weaknesses in the $q models:
while compliant with much older versions of the promises-aplus spec, it is nowhere near complaint with the present 2.x spec and test suites. Since the 2.x suites are focused, in particular about integration of promises with foreign "thenable" libraries, this is actually pretty important.
$q promises are very heavyweight objects, taking more than 750 bytes of heap space per defer/promise pair. The main culprit here is that each promise contains a repeated and identical copy of all the functions used in the closure. This is one of the reason why most major promise implementations use prototypes in one way or the other to reduce the footprint. A flyweight version so implemented usually uses less than 10-50 bytes per instance.
There are a few features that are meaningful conveniences that require no space.
As understood, the primary motivations for the existing model is compactness and simplicity. I believe these features can be maintained while bringing $q up to date and flyweight, but it would require a major restructuring.
I'm inclined to volunteer for the project, but am curious if anybody else is looking at the issue, or wishes to chime in on requirements. Mine are:
identical API, perhaps with additional features (like a resolver instance for promise creation);
Flyweight implementation;
Conforms to most current promises-aplus specification (and hence far better integration with foreign thenables).
The text was updated successfully, but these errors were encountered: