Call for help on making D.js compliant to latest spec

[malko]

Hi, I'm the author of D.js a tiny promise implementation that try to be compliant with the specs. After many attempts I'm still stuck with some (60) failing tests of the latest specs.

Here's a gist of the failing tests report: https://gist.github.com/malko/7798195

As suggested by Domenic i'm asking for help here, so if you can give me any hint i'll be glad to add your name to the contributors list.

To launch the test don't use the adapter but rather use the command: promises-aplus-tests lib/D.js or if you install test suite as D.js dependencies: ./node_modules/promises-aplus-tests/lib/cli.js lib/D.js

Thanks to anyone that will at least have a look at it.

@domenic thank's for the great job with the specs and the test material, and also for your mail answer given earlier.

[wizardwerdna]

Ok, I nailed 32 of them off the top. In _resolve, you correctly pick off the "then" function, and call it on value, passing _resolve and _reject, to with:

then.call(val, _resolve, _reject);

The problem you have is that only one of _resolve and _reject may be called ever, and then at most one time. An evildoer may set up resolution with a nasty thennable that calls both its parameters twice for example, and this is your obligation to protect against under the 1.1 spec.

There are a myriad of ways to "resolve" this problem. One is not to pass the untrusted thennable _resolve and _reject, but specialized versions of them that preclude plural resolution calls. If you define within each closure for the resolution (top of _resolve works fine) the following:

var done;
function once(f){
  return function(x){
    if (done) {
      return undefined;
    } else {
      done = true;
      return f(x);
    }
  }
}

and replace the call at the top of this post with:

then.call(val, once(_resolve), once(_reject));

At most one of _resolve and _reject will be called, and then only one time as required by the spec.

[domenic]

Nice find! In general, my equivalents of _resolve and _reject do a check at their top to ensure they can't be called twice (they bail early if so). This gives the desired semantics for other misuses of them, e.g. if a user does

var p = new Promise(function (resolve, reject) {
   resolve(1);
   resolve(2);
});

which doesn't have anything to do with evil thenables but still could cause double-resolution.

[malko]

@wizardwerdna Thank's a lot, i totally missed that point when resolving with thenable and was searching in totally different ways.

I realize the problem is a little bit harder to solve but you put me in the right track. Please make a pull request to my repository with your proposal so i'll be glad to have you listed as contributor of the project.

[stefanpenner]

this inter maintainer help is awesome. I<3OSS

[wizardwerdna]

OK, malko, I've got you up to spec. The last issue is probably an excellent case in point. The one-time-only execution feature for callbacks not only applies to the two parameters passed to the thenable, but also to the reject in the catch. Changing reject(e) there, to once(_reject)(e) in the catch gets you to an all-green.

That said, there are a few improvements you might wish to make.

The first one is Dom's suggestion to avoid the "once" functor.

The second is that, if you can find a means to detect trustable promises, in particular your own, then you can replace that whole shebang with the equivalent of a simple val.then(fulfill, reject). That avoids the entire process of pulling the then, etc., as well as the need to flatten the thenable for thenable scenarios

A final observation is that your use of the module pattern is a bit heavy for promises, because each promise object contains its own copy of its function properties. Thus, each promise object carries far more data around than the mere queue of promises or pointer to a value/reason. I did not profile your code, but I'll wager that the heap space used per promise is in excess of 500 bytes, when it could easily be under 50. (I did this exercise for angularjs' $q, and it was more than 800 bytes/promise.)

I will throw together a pull request for the changes to go green. The polishing is up to you.

[wizardwerdna]

Here it is: https://github.com/malko/D.js/pull/13

[malko]

ok just merged.

Thanks for your help. I've made some minor modification following your comments. I'll have a deeper look to eventually rethink the _resolve method in a second time.

At least i'm glad that D.js is now fully compliant with the specs, thanks again.