Skip to content

Conditional type is not resolved when used in generics #51080

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
orromis opened this issue Oct 6, 2022 · 6 comments
Closed

Conditional type is not resolved when used in generics #51080

orromis opened this issue Oct 6, 2022 · 6 comments

Comments

@orromis
Copy link

orromis commented Oct 6, 2022

Bug Report

πŸ”Ž Search Terms

conditional types, generics

πŸ•— Version & Regression Information

  • Seems to be broken in every version available in TS playground

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

interface PayloadMap {
    hello: string,
    none: never,
}

type Payload<T extends PayloadMap, K extends keyof T> = T[K] extends never ? [] : [payload: T[K]];

class Base<T extends PayloadMap = PayloadMap>  {
    emit<K extends keyof T>(type: K, ...payload: Payload<T, K>): Base<T> {
        return this;
    }
}

class Implementation<T extends PayloadMap = PayloadMap> extends Base<T> {
    constructor() {
        super();

        this.emit('hello', 'asdf'); // Argument of type '["asdf"]' is not assignable to parameter of type 'Payload<T, "hello">'.(2345)
        this.emit('none'); // Argument of type '[]' is not assignable to parameter of type 'Payload<T, "none">'.(2345)
    }
}

// all following `emit` calls are fine
let impl = {} as Implementation<{test: string} & PayloadMap>;
impl.emit('hello', 'asdf');
impl.emit('none');
impl.emit('test', 'asdf');

class GenericlessImplementation extends Base<{test: string} & PayloadMap> {
    constructor() {
        super();

        this.emit('hello', 'asdf');
        this.emit('none');
        this.emit('test', 'asdf');
    }
}

πŸ™ Actual behavior

This code example doesn't compile and I think it should as the generic is always constraint to PayloadMap.

πŸ™‚ Expected behavior

The example has compile errors because compiler is unable to find out that T is always compatible with PayloadMap.


Some context for the issue I'm dealing with - I'm trying to change typings to a base class we have in our library that is responsible for emitting events and acts as a base class for all other objects that need to emit events in the lib. We have an interface with eventName: PayloadType mapping - it basically defines all events that we emit and what payloads they have. This map is currently static but we want to make it generic so that users of the library can extend it and emit custom events. We need to support events with no payload - those are represneted by the never type. If the payload is never, emit function should be callable only with one argument, if the payload is any other type, the emit function has to be called with 2 arguments - that's what the Payload type does (and it works well - until we introduce generics).

I was looking at this bug report as well but it seems to be about something different, but I'm not 100% sure - #33369.

@MartinJohns
Copy link
Contributor

Resolving of conditional types including unbound type arguments is deferred. This is a design limitation.

Your example would not be type-safe either way, as the type { hello: 'abc', none: never } extends PayloadMap, but the string "asdf" would not be assignable to the type "abc".

@orromis
Copy link
Author

orromis commented Oct 6, 2022

What do you mean by hello: 'abc'? There's only hello: string but maybe I'm missing something.

@MartinJohns
Copy link
Contributor

interface Example { hello: 'abc', none: never }

const imp = new Implementation<Example>()

According to the provided type, the type of the property hello is the string literal "abc", not string.

@orromis
Copy link
Author

orromis commented Oct 6, 2022

But there's no string literal in the playground example or in the original post, there's only this:

interface PayloadMap {  hello: string, none: never }
let impl = new Implementation<{test: string} & PayloadMap>();

The payload map is extended.

Anyway - the main problem is that the type system has a limitation and cannot resolve this? Can it be implemented differently? I just want to avoid having 2 overloads for the emit method as that would mean that events with payloads can be emitted without it (which is something we don't want).

This is what I want to avoid:

emit<K extends keyof T>(type: K): Base<T>;
emit<K extends keyof T>(type: K, payload?: T[K]): Base<T> {
 // emit event
}

Because this call should be invalid:

impl.emit('hello');

@MartinJohns
Copy link
Contributor

MartinJohns commented Oct 6, 2022

Extending PayloadMap doesn't mean the property hello will have the type string, because the extending type may provide a more specific type that extends string. I gave you an example why your code can't work safely. There's simply no way to know that the string "asdf" is valid.

Your second example works, because the type argument is not unbound anymore at the point where you call emit(). The compiler doesn't has to wonder anymore what T exactly is - it now knows, so it can resolve the conditional type.

@orromis
Copy link
Author

orromis commented Oct 6, 2022

Omg, thanks! I didn't realize that you are extending the original type, now I can see where is the problem. Guess I can close the issue now.

@orromis orromis closed this as completed Oct 6, 2022
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

No branches or pull requests

2 participants