whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.1k stars 2.66k forks source link

Promises: Clarify the type of the objects in HostPromiseRejectionTracker and associated sets #10721

Open ADKaster opened 1 day ago

ADKaster commented 1 day ago

What is the issue with the HTML Standard?

Going off of discussion in the tc39 matrix and issues like https://github.com/whatwg/html/issues/676 and https://github.com/whatwg/html/issues/7591, it seems like the expectation for implementers is that events like PromiseRejectionEvent are supposed to contain a reference to the actual JS promise that was rejected.

Does this mean that we cannot perform any kind of JS <-> WebIDL conversion of promises like in https://webidl.spec.whatwg.org/#js-promise, where WebIDL asks us to create a new promise capability resolved with the JS value.

Currently the outstanding rejected promises weak set and the About to be notified rejected promises list state that they contain Promise, pointing to https://webidl.spec.whatwg.org/#idl-promise.

One reading of this type would be that types stored in the two sets should be WebIDL promises, as converted from JS values on entry to HostPromiseRejectionTracker. And, that the PromiseRejectionEventInit for events from those promises would also contain a PromiseCapability resolved with the actual JS promise.

Is this correct? It sounds from the other references I've seen like that's not the intent of PromiseRejectionEvent and those two sets. In that case, the two sets and PromiseRejectionEvent should not point to https://webidl.spec.whatwg.org/#idl-promise with its implied conversion of JS values to PromiseCapabilites, but instead invent some lower-case 'promise' concept to convey that we want the actual JS value here? That makes the IDL for PromiseRejectionEvent a bit awkward though.

ADKaster commented 1 day ago

Curiously, this also leads to a difference in how Firefox behaves compared to Safari and Chrome.

Firefox:

image

Everyone else accepts '42' as the promise value in PromiseRejectionEventInit.

domenic commented 23 hours ago

Does this mean that we cannot perform any kind of JS <-> WebIDL conversion of promises like in https://webidl.spec.whatwg.org/#js-promise, where WebIDL asks us to create a new promise capability resolved with the JS value.

This is the intent.

Currently the outstanding rejected promises weak set and the About to be notified rejected promises list state that they contain Promise, pointing to https://webidl.spec.whatwg.org/#idl-promise.

They should probably point to JS promise instead. This kind of fuziness between IDL and JS concepts shows up often in web specs, since usually it doesn't matter. You're right that in the case of Web IDL Promise<T> vs. JS Promise, it does matter.

Curiously, this also leads to a difference in how Firefox behaves compared to Safari and Chrome.

This is a symptom of https://github.com/whatwg/html/pull/9897, which I suspect Firefox implemented (since they opened https://github.com/whatwg/streams/issues/1298#issuecomment-1774668957).

There are three desiderata in the design of PromiseRejectionEvent:

  1. Do not add any custom bindings, or customizations to the generic event constructor spec.
  2. Have (new PromiseRejectionEvent('foo', { promise: p })).promise === p
  3. Have new PromiseRejectionEvent('foo', { promise: v }) treat v like the rest of the platform does, by wrapping non-promise values into promises.

I cannot find a way to accomplish all three of these at once, and (3) seems like the least important. So I like the current spec and Firefox's behavior here the best. Adding web platform tests for it, if we don't have them already, would be very nice.

annevk commented 12 hours ago

Safari and Chrome accomplish 2 when p is an actual promise and I don't think they have custom bindings for this class. Perhaps IDL should be updated to not wrap when it isn't needed?

https://software.hixie.ch/utilities/js/live-dom-viewer/?%3Cscript%3E%0Ap%20%3D%20Promise.resolve()%3B%0Aw((new%20PromiseRejectionEvent(%27foo%27%2C%20%7B%20promise%3A%20p%20%7D)).promise%20%3D%3D%3D%20p)%0A%3C%2Fscript%3E