Yahweasel / libav.js

This is a compilation of the libraries associated with handling audio and video in ffmpeg—libavformat, libavcodec, libavfilter, libavutil, libswresample, and libswscale—for emscripten, and thus the web.
288 stars 18 forks source link

Fix `LibAV()` overload for typescript. #43

Closed reinhrst closed 6 months ago

reinhrst commented 6 months ago

Typescript's overload system is no smart enough to realise that if a function overload exists for two types, the function can also be called on the union of those two types.

Therefore if only the following is defined:

LibAV(opts?: LibAVOpts & {noworker?: false}): Promise<LibAV>;
LibAV(opts: LibAVOpts & {noworker: true}): Promise<LibAV & LibAVSync>;

the following call will fail:

let opts: LibAVOpts = ...
const libav = LibAV.LibAV(opts)

I'm not 100% satisfied with this solution, and there might be some way to fix it with generics, but I didn't manage. The solution was tested on all code below (none of my other solutions worked for all 5 cases):

const _libav0 = await window.LibAV.LibAV()
const _libav1 = await window.LibAV.LibAV({})
const _libav2 = await window.LibAV.LibAV({noworker: false})
const _libav3 = await window.LibAV.LibAV({noworker: true})
const _libav4 = await window.LibAV.LibAV(libavoptions as LibAVOpts)
Yahweasel commented 6 months ago

The thought process that led me to think that what's there was sufficient is that the first case is the union of both cases, since the intersected field is optional anyway. But, I guess {noworker?: bool} & {noworker?: false} is {noworker?: false}? In retrospect, the code wouldn't have worked at all if {noworker?: bool} & {noworker?: false} is {noworker?: bool}, and that makes no type-theoretical sense either...

Yahweasel commented 6 months ago

A couple questions:

(1) Among the solutions you tried, was one of them changing the noworker?: false overload to be

    LibAV(opts?: LibAVOpts): Promise<LibAV>;

(i.e., just remove the noworker intersection entirely), and if so, how did TypeScript break that?

(2) Surely the type LibAV | (LibAV & LibAVSync) is equal to the type LibAV? What does this union do?

reinhrst commented 6 months ago

A couple questions:

(1) Among the solutions you tried, was one of them changing the noworker?: false overload to be

    LibAV(opts?: LibAVOpts): Promise<LibAV>;

(i.e., just remove the noworker intersection entirely), and if so, how did TypeScript break that?

(2) Surely the type LibAV | (LibAV & LibAVSync) is equal to the type LibAV? What does this union do?

I would say that if (2), then indeed one could do (1)... It took me a while time to convince myself, but I now think that LibAV | (LibAV & LibAVSync) != LibAv.

Example

type A = {a: number}
type BC = {b: number, c: number}

const a = "dummy" as unknown as A
const aabc = "dummy" as unknown as A | (A & BC)

if ("b" in aabc) {
    console.log(aabc.c)
}

if ("b" in a) {
    console.log(a.c)  // <-- this gives an error: Property 'c' does not exist on type 'A & Record<"b", unknown>'.
}

See here in playground.

So basically, if you get a LibAV | (LibAV & LibAVSync) as a type, you can narrow it down with one check to LibAV & LibAVSync.

Using interfaces instead of type results in the same behaviour.

BTW I expect that the same issues arise with the overloads I added last week in #41 .... I guess it's worth fixing once I run into them :)

Yahweasel commented 6 months ago

Took me some mental convincing, but I think the situation is basically this: LibAV | (LibAV & LibAVSync) is, type-theoretically, the same as LibAV, but that doesn't mean that it's the same for the purpose of narrowing and other uses. I'm convinced that this is the rightest way.