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

Fix LibAV() overload for typescript. #43

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

reinhrst
Copy link
Contributor

@reinhrst reinhrst commented Dec 9, 2023

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)

Typescript's overload system is [no smart
enough](https://www.typescriptlang.org/docs/handbook/2/functions.html#writing-good-overloads)
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:

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

the following call will fail:

```ts
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):

```ts
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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Contributor Author

reinhrst commented Dec 9, 2023

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
Copy link
Owner

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.

@Yahweasel Yahweasel merged commit f1dbedc into Yahweasel:master Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants