Skip to content

function ...args with generic union narrows incorrectly after second param #31413

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
ChuckJonas opened this issue May 15, 2019 · 5 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@ChuckJonas
Copy link

ChuckJonas commented May 15, 2019

TypeScript Version: 3.4.x (latest ts-playground)

Search Terms:
...args with generic to narrow

Code

//setup
type QueryField<T> = keyof T; 

interface FunctionField<T> {
    fn: 'count' | 'avg';
    alias?: string;
    field: QueryField<T>;
}

class fieldBuilder<T>{
    public select<F extends FunctionField<T>, P extends QueryField<T>>(...args: Array<P | F>) {}
}


//example error
interface Foo{
    x: string;
    y: number;
    z: boolean;
}

let builder = new fieldBuilder<Foo>();

//works
builder.select('x', 'y', 'z'); 

//Type Error: Argument of type '"y"' is not assignable to parameter of type '"x" | FunctionField<Foo>'.
builder.select({ fn: 'count', field: 'x' }, 'x', 'y'); 

Expected behavior:

I would expect that my ...args array could be any number of items from FunctionField<T> or QueryField<T>...

Actual behavior:
As soon as I mix the types, it starts to narrow the QueryField<T> based on the first type passed it.

Playground Link:
Link

Related Issues:

@ChuckJonas
Copy link
Author

Also worth note, I only started seeing this issue once I upgraded from 3.3.1 -> 3.4.x

@ChuckJonas
Copy link
Author

also, I know the MRE doesn't make sense, but it's the simplest way I could reproduce the error

@ahejlsberg
Copy link
Member

Before I address the issue, let me ask a question. Are F and P used anywhere else (such as in the return type) in the signature?

class fieldBuilder<T>{
    public select<F extends FunctionField<T>, P extends QueryField<T>>(...args: Array<P | F>) {}
}

If not, why not write the signature like this:

class fieldBuilder<T>{
    public select(...args: Array<QueryField<T> | FunctionField<T>>) {}
}

In general, type parameters should always be kept to a minimum. It is suspicious to have type parameters that don't occur both in an input position (i.e. parameter) and an output position (i.e. return type or callback parameter). And it is particularly suspicious to have type parameters that only occur once in a signature (since you might as well replace them with their constraint).

@ChuckJonas
Copy link
Author

@ahejlsberg I came to that realization after submitting this (which is part of the reason I said the MRE doesn't really make sense). Just typing the parameters directly does fix the issue.

I didn't close it because it still seemed like a bug? If not, feel free to close this out

@ahejlsberg
Copy link
Member

So, the reason it worked in earlier versions is because we failed to make any inferences for P and F and just reverted to the constraints. With #29847 we now do make inferences for P and F. But because there is nothing discerning about P vs. F (other than constraints which aren't considered until after inference is finished) the inferences end up being poor. So, we now fail in a slightly different manner that causes an error. But, as I alluded to above, the type parameters should just be eliminated.

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

2 participants