$q is probably due for a rewrite

[wizardwerdna]

Given the importance of promises to the angularJS way, it is probably worth spending some time considering weaknesses in the $q models:

1. 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.

2. $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.

3. 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:

1. identical API, perhaps with additional features (like a resolver instance for promise creation);

2. Flyweight implementation;

3. Conforms to most current promises-aplus specification (and hence far better integration with foreign thenables).

[petebacondarwin]

Can you provide a link to the 2.x suite of tests and perhaps the result of running $q against it?

[wizardwerdna]

Its all at https://github.com/promises-aplus/promises-tests. The easiest
way would be to delete node_modules/promises-aplus...., modify package.json
version number to "*" and run a fresh npm install.

Once you do that, run the tests using grunt, but before you do, be sure to
update the "promises-aplus" adapter, the form of which has been modified.
(Essentially, changing fulfill and fulfilled to resolve and resolved -- I
set up my adapters to run with both forms of the suite.)

Before you change the adapter, every test will fail. After you change the
adapter, a kazillion tests will fail. The key changes that account for
most of the errors are:

(1) callbacks must be called with the global object in loosey-goosey mode
and undefined in strict mode as this.
(2) Foreign thenables must be especially handled and called in the resolve
process, per the new 1.1 promises-aplus spec. The process entails: (i)
assuring that promise.then is called exactly one time for each potential
promise object (to protect against thenables with getters and setters
returning different values; (ii) assuring that the "then" is called with
resolve/reject for the handling promise in a particular way, and that at
most 1 of resolve/reject is called, and then at most one time (this is
trickier than it seems); and (iii) the foreign thenables are flattened
until a value is reached (possibly resulting in a loop in willingly
obnoxious cases). For (iii), you are entitled to attempt loop detection
and MAY reject in that case, but you are required to handle self-resolution
(for both native and foreign promises), and MUST throw a type-error in that
case.

To that end, efficiency demands that you handle native promises separately,
since all they require is performing a p2 = p.then(p.resolve, p.reject)
(with appropriate bindings) and you are done with the resolve.

The current $q is a bit of a mess with an eye toward those changes. I am
already working on a refactoring for that, which has several benefits: (i)
it has a single core object which does not include all the code in each
instance; and (ii) permits a decent separation of concerns, permitting
refactoring of repetitive common code.

For obvious reasons, I am retaining so much of the existing code as I can,
although a total rewrite would be easier (when isn't it?) The external API
will (as it must) be identical (I am adding one function right now) with
identical semantics, and the existing tests will all pass.

Once I have the refactoring done to satisfy the existing test suite, I will
start the 1.1 (2.x test suite) changes. I've done a few suites like this
already (covenant and D.js), and think that once refactored, the upgrade
should be simple.

Current footprint is more than 750 bytes of stack per promise, and should
be reduced to less than 50 with this approach.

On Fri, Dec 6, 2013 at 2:28 PM, Pete Bacon Darwin
notifications@github.comwrote:

Can you provide a link to the 2.x suite of tests and perhaps the result of
running $q against it?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5223#issuecomment-30035039
.

[caitp]

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 :(

[wizardwerdna]

I can pass along some guidance as to key areas to give attention. In the
2.x suite there are many tests for each issue, so you might only have a few
failing issues:

1. must handle foreign thenables in a very particularized way
   a) must call the then at most one time, because the then could be a
   getter. So you must save the result of the get.
   b) must assure that the callbacks/errbacks to the then, only one is ever
   called, and then at most once. They do CRAZY things to senak around this,
   so you must code carefully. For starters, I would recommend writing a
   functor that takes each callback and errback and buries it in a function (i
   called mine once) that assures at most one call. (Check out the code in
   Covenant for an example). Not only must you assure at most once to the
   callback and errback, but you must also assure that this applies to the
   failure throw case as well.
   c) the code for the foreign thenable handling is expensive and slow, so
   you will want to identify trusted promises, which will require a bit of a
   trick from the existing code, which doesn't really have constructors to
   identfy same. If you can distinguish them, you can do the entire resolve
   by simply executing d.resolve(p);
   d) you MUST throw an error for d.resolve(d.promise)

If you nail these four issues, I would be surprised if you aren't finished.
Indeed, if you think you have messages not related to them, I would read
carefully to make sure (as the messages are pretty weak).

If you would like another set of eyes, please advise. I'll sit back on the
project until you weigh in.

On Mon, Dec 16, 2013 at 12:44 PM, Caitlin Potter
notifications@github.comwrote:

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 :(

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5223#issuecomment-30698416
.

[caitp]

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.

[wizardwerdna]

Will do. I tried that approach iniitally, and decided it needed a serious
refactoring before meaningful change could be effected. Let's try it your
way again, now that i've done some work on its internals.

On Mon, Dec 16, 2013 at 4:19 PM, Caitlin Potter notifications@github.comwrote:

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

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5223#issuecomment-30715233
.

[hswolff]

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?

[caitp]

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)

[wizardwerdna]

Its a fine place to begin the discussion. Yes, native promises are coming,
and they are largely Promises/A+ 1.1 compatible. So bringing $q up to
speed would be a big win. Also yes, there is a different API presented,
but they are trivially matched with an adapter. For example, you can build
a convenience function for defer whenever you new up a traditional promise
object.

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
90% home.

On Mon, Dec 16, 2013 at 7:03 PM, Caitlin Potter notifications@github.comwrote:

We've talked about that a bit today @hswolff https://github.com/hswolff,
but I guess it's probably going to be a while before we can really relegate
things to es6 functionality (although definitely, that would be awesome)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5223#issuecomment-30722677
.

[ewinslow]

Related to native promises: Wondering if Angular would ever support a similar type of API:

var promise = $q.promise(function(resolve, reject) {
  //... 
});

[wizardwerdna]

@ewins I'm a fan of the resolver function promise API.

[caitp]

@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

[wizardwerdna]

I will do. Find it odd that we are in this situation -- my experience is
that a tight core makes the rest simpler. I'll do some surgery this week.

Best.

On Thu, Dec 26, 2013 at 11:35 AM, Caitlin Potter
notifications@github.comwrote:

@wizardwerdna https://github.com/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

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5223#issuecomment-31232668
.

[caitp]

yeah, it's a bit weird. It only seems to break the finally tests, calling the handlers too soon I guess