Skip to content

Union of subtypes should be usable as the supertype #38048

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

Closed
wojpawlik opened this issue Apr 19, 2020 · 5 comments
Closed

Union of subtypes should be usable as the supertype #38048

wojpawlik opened this issue Apr 19, 2020 · 5 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@wojpawlik
Copy link

TypeScript Version: 3.8.3 and v3.9.0-dev.20200418

Search Terms:
common type subtype union

Code

interface Cata<OkIn, OkOut, ErrIn, ErrOut> {
    Ok(value: OkIn): OkOut
    Err(err: ErrIn): ErrOut
}

type CataOk<OkIn, OkOut> = Cata<OkIn, OkOut, never, unknown>
type CataErr<ErrIn, ErrOut> = Cata<never, unknown, ErrIn, ErrOut>

interface BaseResult<T, E> {
    readonly ok: boolean
    cata<OkOut, ErrOut>(cata: Cata<T, OkOut, E, ErrOut>): OkOut | ErrOut
}
export class Ok<T> implements BaseResult<T, never> {
    readonly ok = true as const
    constructor(readonly value: T) {}
    cata<OkOut>(cata: CataOk<T, OkOut>) {
        return cata.Ok(this.value)
    }
}
export class Err<E> implements BaseResult<never, E> {
    readonly ok = false as const
    constructor(readonly error: E) {}
    cata<ErrOut>(cata: CataErr<E, ErrOut>) {
        return cata.Err(this.error)
    }
}

export type Result<T, E> = Ok<T> | Err<E>

// tests

declare const u: Result<number, Error>

u.cata({
    Ok(v) { return '2' },
    Err() { return 3 },
})

Expected behavior:

No error (Liskov substitution principle).

Actual behavior:

This expression is not callable.
Each member of the union type (<OkOut>(cata: CataOk<number, OkOut>) => OkOut) | (<ErrOut>(cata: CataErr<Error, ErrOut>) => ErrOut) has signatures, but none of those signatures are compatible with each other. ts(2349)

Workaround:

export type Result<T, E> = BaseResult<T, E> & (Ok<T> | Err<E>)

Playground Link

Related Issues: #37704.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Apr 20, 2020
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.0 milestone Apr 20, 2020
@weswigham
Copy link
Member

Our rules for calls on unions are quite strict, as we do not have the capability to "resolve the call on each union member". When attempting to call a union, the call is only allowed of the signatures are sufficiently similar such that we can unify them into a single signature.

In this case we cannot, because each of the input signatures has a different type parameter (declared in a different location), which is going to prevent us from merging them. (While we could combine the parameter list, making inference then behave the same way on the input arguments as the component signatures isn't something we can do - the combined parameter type is inferred to by a potentially different set of inference heuristics).

The intersection with the base type workaround succeeds, as then in the resulting union, each type shares a common overload (the one intersected from the base), which we can trivially resolve against. It's worth noting that class inheritance works differently than intersection here - intersection merges signature lists, while when subtyping a class, you override them.

@wojpawlik
Copy link
Author

Thank you for your thorough answer.

each of the input signatures has a different type parameter (declared in a different location)

Are you referring to generic type or its instantiation? If the former, I disagree that it depends on location of the declaration:

type AnyFn = (x: never) => unknown

interface Cata<OkFn extends AnyFn, ErrFn extends AnyFn> {
    Ok: OkFn
    Err: ErrFn
}

interface BaseResult<T, E> {
    readonly ok: boolean
    cata<ValueOut, ErrorOut>(
        cata: Cata<(x: T) => ValueOut, (x: E) => ErrorOut>
    ): ValueOut | ErrorOut
}

export class Ok<T> implements BaseResult<T, never> {
    readonly ok = true as const
    constructor(readonly value: T) {}
    cata<Out>(cata: Cata<(value: T) => Out, AnyFn>) {
        return cata.Ok(this.value)
    }
}
export class Err<E> implements BaseResult<never, E> {
    readonly ok = false as const
    constructor(readonly error: E) {}
    cata<Out>(cata: Cata<AnyFn, (error: E) => Out>) {
        return cata.Err(this.error)
    }
}

export type Result<T, E> = Ok<T> | Err<E>

// tests

declare const v: Result<number, Error>

const o = v.cata({
    Ok(v) {
        return "2"
    },
    Err() {
        return 3
    },
})

Playground

This code (evolved in the meantime) doesn't alias Cata, yet it hits analogous error.

@weswigham
Copy link
Member

Are you referring to generic type

The generic type parameter on the method itself is to what I am referring. Each method declaration has a distinct list of them.

@zlepper
Copy link

zlepper commented May 6, 2020

I find that example really difficult to read, but i assume it's basically the same as what is happening here:

interface Base {
    foo: string;
}

interface SpecificA extends Base {
    bar: number;
}

interface SpecificB extends Base {
    baz: boolean;
}

enum TheType {
    A,
    B
}

interface A {
    type: TheType.A;
    foo: SpecificA[];
}

interface B {
    type: TheType.B;
    foo: SpecificB[];
}

type options = A | B;


// tests

function getValue(): options {
    return {
        type: TheType.A,
        foo: [
            {
                bar: 42,
                foo: 'my value'
            }
        ]
    };
}

const value = getValue();

value.foo.map(v => console.log(v.foo));

where the .map call is impossible, even though accessing v.foo should certainly be allowed.

I'll try to work around it by doing a bunch of type casting.

@wojpawlik
Copy link
Author

I encountered a scenario in which the requested behavior is undesirable:

interface Base {
	type: string
}

interface Foo extends Base {
	type: 'foo'
}
interface Bar extends Base {
	type: 'bar'
}

type Foobar = Foo | Bar
type FoobarTypeProp = Foobar['type'] // should be `'foo' | 'bar'`, not `string`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants