jsdom / webidl2js

Auto-generate JS class structures for Web IDL specifications
MIT License
79 stars 30 forks source link

Fix ECMAScript to IDL `Promise` type conversion #219

Closed ExE-Boss closed 4 years ago

ExE-Boss commented 4 years ago

Most notably, this fixes the bug where rejected promises were being converted into resolved promises and that Promise.resolve(x) returns x when x is a Promise object from the same realm as the Promise.resolve function, whereas WebIDL requires the conversion to always return a new Promise object.


Note that there still is a small timing difference for non‑void promises due to the fact that the IDL conversion and promise reaction are being performed in separate then callbacks, whereas WebIDL runs them in the same then callback. (See the “to react to a Promise<T> algorithm)

Fixing this minor timing discrepancy would require the type conversion to be performed manually in implementation classes or overriding the then method of the converted Promise instance.


P.S.: If you still want then to be called with two arguments, then the correct callback is:

reason => {
    throw reason;
}

Which is the default behaviour when null or undefined is passed.

domenic commented 4 years ago

Can you remove the T conversion entirely? As I pointed out, it does not exist in the Web IDL spec.

ExE-Boss commented 4 years ago

It does exist in the “to react to a Promise<T> algorithm.

domenic commented 4 years ago

Right. When the spec reacts to it, it can do the conversion. We shouldn't do it automatically, especially since nobody reacts to return types.

domenic commented 4 years ago

I'm sorry, perhaps I wasn't clear enough. The type conversion logic should not live in webidl2js at all.