Uncovered code as of 07-Jun-14: #3 - typeof x !== 'function' guards about native-promise-only

[getify]

The guards in checkCapability (lines 217-221)

The reason for those checks is... well... quite complicated.

If you want to dive deep down the rabbit hole, check out these threads:

• domenic/promises-unwrapping#107
• domenic/promises-unwrapping#24 (really really long!)

As for how to devise a test that covers them, see that checkCapability is called from Promise.resolve(..) Promise.reject(..) Promise.all(..) Promise.race(..) -- and then they each call a Constructor that comes the this binding to construct a promise to use. This is mandated by the spec.

If you did Promise.resolve.call( someOtherPromiseConstructor, ... ), that would substitute in a different this, thus a different Constructor, and you'd have to assume you couldn't trust the resolve and reject capabilities that the other Constructor provides. The discussion threads above (especially the first one) explain why we have to treat the failed capabilities as a sync thrown error rather than deferring that check until "later" at the call-site.

To test, we could do something like:

function someOtherPromiseConstructor(executor) {
   executor( "foo", "bar" );
}

try {
   Promise.resolve.call( someOtherPromiseConstructor, 42 );
   Promise.reject.call( someOtherPromiseConstructor, 42);
   Promise.all.call( someOtherPromiseConstructor, [ Promise.resolve(42) ] );
   Promise.all.race( someOtherPromiseConstructor, [ Promise.resolve(42) ] );
}
catch (err) { .. }

Make sense?

There's one other code path that checkCapability(..) is called on: line 254.

then(..) produces its own promise for promise-chaining. Technically, the spec implies (doesn't state explicitly but doesn't disallow) you should be able to do this (though almost the only reason for such craziness is sub-classing, which is dubious as stated in other threads):

var p = new Promise(..);
var aDifferentP = p.then.call( someOtherPromiseConstructor, .. );

My implementation doesn't support that currently, because before doing some perf optimizations, it was impossible -- now it's possible but not supported. So I could for consistency sake go back and change line 253 to new this.constructor(..) instead of new Promise(..).

It would enable such a test for then(..) this-borrowing to pass, but again I'm dubious (especially given the "inherited" .constructor junk), because there's almost no justifiable case for that kind of thing except for sub-classing. Shrugs.

I'll probably make a little patch nonetheless, even though I think it's a silly and mostly moot feature.

[getify]

[Update]: In the process above of changing to new this.constructor(..), found another bug where all along I've been setting Promise.prototype incorrectly. So, yay for finding and fixing bugs!

[getify]

The test for the then(..) this-borrowing and checkCapability should be something like:

try {
   Promise.resolve(42).then.call( someOtherPromiseConstructor );
}
catch (err) { .. }

[getify]

Forgot that this-borrowing also needs to work for the catch(..) method. Fixed that. The test would be:

try {
   Promise.reject(42).catch.call( someOtherPromiseConstructor );
}
catch (err) { .. }

[getify]

closing for now because I believe these issues have all been addressed. re-open if not.