In 25.4.5.3, Promise.prototype.then, the resolve handler is defaulted with the following steps:
7. If IsCallable(onRejected) is true, then
a. Let rejectionHandler be onRejected.
8. Else,
a. Let rejectionHandler be a new Thrower Function (see 25.4.5.3.3).
This implies that not just undefined will be replaced by the default function, but arbitrary arguments, e.g. 65. That seems both weird and inconsistent with the behaviour of default arguments of ordinary JS functions. I think the logic should be:
7. If onRejected is undefined, then
a. Let rejectionHandler be a new Thrower Function (see 25.4.5.3.3).
8. Else, if IsCallable(onRejected) is false, then
a. Throw a TypeError
8'. Else,
a. Let rejectionHandler be onRejected.
Similarly for steps 9-10 handling the reject handler.
Around seven months ago it was decided to use `JSON.parse`/`JSON.stringify`-style optional functions instead of `Array.prototype.sort`-style:
http://esdiscuss.org/topic/promises-consensus-with-a-terminology#content-10
This feature has been present in all drafts presented since then and has not seen objections until now. It is shipping in Blink and Firefox.
Throwing synchronously as a behavior of `then` would be horrible, in any case; returning a rejected promise is more coherent, as noted in the above thread.
Some previous discussion in Promises/A+ space:
https://github.com/promises-aplus/promises-spec/pull/32#issuecomment-10335630
A agree with Andreas on this. Treating any non-function as a request to use the default is not idiomatic ES, even if JSON.stringify/parse work that way. We should have sanitized those in ES5, but missed it.
I also don't agree that the associated error (if any) should be asynchronous. Passing a non-function in those argument positions is an error relating to the the then call site rather than an error related to the deferred action:
aPromise.then(new Array, new Map)// oops passing the wrong things to 'then'
//should be an immediate exception.
There are already other synchronous exceptions: steps 4 and 6 in Rev22 draft of 'then'. A wrong argument type exception would be comparable in nature to those.
Finally, I don't think what FF and Blink is particularly relevant. In TC39 we try not to let early implementation choices prevent us from spec'ing the right thing.
The other precedence for IsCallable checks is valueOf, toString, etc. Somewhat different, but still.
Literally the only place where non-callable arguments result in a thrown error is Array.prototype.sort.
Anyway this would break tons of promise code in the wild that is using falsy sentinels (`null` and `false` in particular). So you would have to special-case those two at the very least.
(In reply to comment #3)
>
> Anyway this would break tons of promise code in the wild that is using falsy
> sentinels (`null` and `false` in particular). So you would have to special-case
> those two at the very least.
Really?? Do you have examples? (I'm not questioning your assertion, I just curious to see why somebody would do that.)
Lots of examples on GitHub with https://github.com/search?l=javascript&q=%2B%22.then%28null%22&ref=searchresults&type=Code; hard to narrow down which of those are using native promises, if any.
First of all, code like
p.then(null, handler)
is extremely common in idiomatic promise code.
Second of all, we as TC39 need to respect the fact that promises started out in DOM-land and have already landed in both Chrome and Firefox. Moreover, the January changes are causing genuine hardship for Chrome implementors. I understand that ES6 isn't completely finalized, but promises are a high-visibility, highly desired feature, and are already implemented in two browsers. It's well past time for changes that break common development idioms.
Dave
(In reply to comment #6)
> First of all, code like
>
> p.then(null, handler)
>
> is extremely common in idiomatic promise code.
>
And that makes sense coming from DOM land (and even arguably in s pure JS context). But do we really want to enable:
p.then({a:42}, handler);
Do we think anybody ever coded that where it wasn't a misunderstanding of the then API? Allowing it is obscuring a bug.
> Second of all, we as TC39 need to respect the fact that promises started out in
> DOM-land and have already landed in both Chrome and Firefox. Moreover, the
> January changes are causing genuine hardship for Chrome implementors. I
> understand that ES6 isn't completely finalized, but promises are a
> high-visibility, highly desired feature, and are already implemented in two
> browsers. It's well past time for changes that break common development idioms.
Well, any webidl based API definition would throw an exception if an non-callable object was passed to a parameter position that expected callback. And, it was Andreas Rossberg who opened this bug.
Clearly null needs to be accepted. But it is a lot less clear that other numbers, strings, booleans, and non-callable objects should be accepted or that rejecting them would breaking anythng.
I agree it's fine to special-case 'null' for legacy compatibility. But as Allen said, it is difficult to imagine any other case that is not actually a bug. (FWIW, I just searched some representative patterns on both GitHub and Google's internal CodeSearch, and neither did come up with anything.)
(In reply to comment #8)
> I agree it's fine to special-case 'null' for legacy compatibility. But as Allen
> said, it is difficult to imagine any other case that is not actually a bug.
> (FWIW, I just searched some representative patterns on both GitHub and Google's
> internal CodeSearch, and neither did come up with anything.)
I also did some searching and couldn't find any on github. I'm going to only allow undefined and null unless somebody can find real examples where values other than null or undefined are used as defaulting values.
I don't care much which values are allowed, but I feel strongly that when interfacing with asynchronous APIs, the errors given should be asynchronous. Otherwise, programmers cannot isolate errors reliably in sections of their code using a single catching technique, and they are forced to double-wrap code in .catch() and try { } catch { }.
Using promise APIs to isolate errors is one of their primary use cases. Trying to distinguish between "an error relating to the the then call site rather than an error related to the deferred action" is fruitless: no APIs distinguish this way currently. If you do
try {
doSomethingSync(badArg);
} catch (e) {
// isolate e from other parts of my app, allowing them to continue
}
there is no exception for the fact that badArg is a "call-site error"; it gets caught anyway. Similarly, if you do
doSomethingAsync(badArg).catch(e => {
// isolate e from other parts of my app, allowing them to continue
});
you would not expect the error to escape your catch and bubble up to other parts of the program, or possibly crash your server if you are using something like Node.js.
---
I am strongly against this consensus-breaking change, as I believe are Mark and Yehuda. It should not be done.
TC39 at its April 2014 meeting agreed to treat all non-callable then arguments as defaulting values
changed in rev24 editor's draft
fixed in rev24 editor's draft
fixed in rev24