Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"React to a promise" is not equivalent to Promise.prototype.then with missing handlers #921

Closed
andreubotella opened this issue Sep 17, 2020 · 2 comments · Fixed by #922
Closed

Comments

@andreubotella
Copy link
Member

The "react to a promise" algorithm seems to be a spec-level version of Promise.prototype.then, except that when it's called with a missing handler, a default handler is substituted which simply resolves the promise with undefined.

In other words, you would expect "react to a promise" to be equivalent to:

// promise: Promise<T>
// onFulfillment?: (T) => U
// onRejection?: (any) => U
promise.then(onFulfillment, onRejection) // Promise<U>

and it's instead:

promise.then(onFulfillment ?? () => undefined, onRejection ?? () => undefined) // Promise<U | undefined>

I understand that, even when there's no fulfillment handler given, the conversion to IDL of the resolved value must happen, so the promise can reject if the conversion throws (see also #782). But even in that case, the return value shouldn't be undefined. And for the rejection handler even that is unnecessary, since converting an ECMAScript value to any cannot fail.

The fact that this is the intended behavior is borne out by the definitions of "upon fulfillment" and "upon rejection", as well as by the usage of these operations in the Fetch, Streams, Background Fetch and Service Worker specs.

@domenic
Copy link
Member

domenic commented Sep 17, 2020

Yeah, this seems pretty bad. Are you interested in working on a PR to fix this?

@andreubotella
Copy link
Member Author

andreubotella commented Sep 17, 2020

Yeah, this seems pretty bad. Are you interested in working on a PR to fix this?

Sure!

andreubotella pushed a commit to andreubotella/webidl that referenced this issue Sep 17, 2020
"React to a promise"'s default handlers used to return undefined rather
than propagating the resolved value or the rejection reason to the
returned promise. This change fixes that, and changes "upon fulfillment"
and "upon rejection" to return the resulting promise.

Closes whatwg#921.
domenic pushed a commit that referenced this issue Sep 18, 2020
"React to a promise"'s default handlers used to return undefined rather
than propagating the resolved value or the rejection reason to the
returned promise. This change fixes that, and changes "upon fulfillment"
and "upon rejection" to return the resulting promise.

Closes #921.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants