Consistently use Promise<void> instead of Promise<undefined>

[lucacasonato]

lib.dom.d.ts ubiquitously uses Promise<void> for promises not resolving to undefined. It has done this for a long time.

When the generator was written, there were 0 or 1 (I can't quite tell) places where this type of promise occured outside of a function return value. However, Web Streams are now widely supported, and they have properties that are promises that resolve to no value. These are currently typed as Promise<undefined>. This commit changes them to be typed as Promise<void>, to align with the rest of lib.dom.d.ts.

There are 5 readonly properties that are impacted by this change.

[github-actions[bot]]

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

[lucacasonato]

@microsoft-github-policy-service agree company="Deno Land Inc"

[saschanaz]

Can you modify the description as it all says just Promise, I believe you intended to say Promise<something>.

I'm not sure this is the right direction btw, shouldn't we do the reverse way and do Promise<undefined> everywhere? cc @sandersn

[lucacasonato]

@saschanaz indeed, had to escape the < and > chars

[saschanaz]

Does this change affect real code btw? Having Promise<undefined> should be okay since https://github.com/microsoft/TypeScript/pull/53092.

[lucacasonato]

It has a compat issue with lib.deno.web.d.ts, as it uses Promise<void> throughout

[sandersn]

In the absract, switching to Promise<undefined> would be the right thing, but both the weight of backward compatibility and the special cases for returning void mean that it's likely to break something.

I'm not at my desk right now but when I get back I'll try an experiment of manually replacing Promise<void> with Promise<undefined> in the std lib. I see 93 uses.

Thinking about it a bit more, most of those 93 uses are the return types of async functions. And I suspect that environments like node and deno have lots more uses like that. So maybe it's better to give these 5 properties worse types for consistency.

[sandersn]

Looking at the results so far, I see three categories of break:

1. Promise<undefined> isn't assignable to explicit type annotations : Promise<void>
2. In an async function, returning a Promise<void> counts as void for the purposes of classifying a function as needing to return a value from each code path. Returning a Promise<undefined> from one branch means that other branches need to return a value as well.
3. Code that extends DOM types in specific ways isn't assignable because the overrides have explicit Promise<void> types. I didn't take time to understand the details; see the DT results.

[saschanaz]

2. In an async function, returning a Promise<void> counts as void for the purposes of classifying a function as needing to return a value from each code path. Returning a Promise<undefined> from one branch means that other branches need to return a value as well.

This sounds like a bug, both should work same, right?

But it does sound like we should compromise here.

[Renegade334]

This raised its head again with microsoft/TypeScript#60987, where certain third-party polyfills (for webcodecs in particular) have properties typed as Promise<void> but where those missing interfaces are now present in the DOM libs with properties typed as Promise<undefined> as per the IDL, causing errors.

Was the verdict here WONTFIX?

[saschanaz]

Err, I think this has been just forgotten. Let's merge this. LGTM.

[github-actions[bot]]

Merging because @saschanaz is a code-owner of all the changes - thanks!

[darksabrefr]

These changes have to be reflected in Node's types too, because they use Promise<undefined>, making them not compatible anymore since TS 5.8 was released : https://github.com/DefinitelyTyped/DefinitelyTyped/blob/515f950351b890f8c45bbfcc793cdc9598da67e8/types/node/stream/web.d.ts#L100

[darksabrefr]

Discussion opened here: https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/72102