Skip to content

Mapped type function argument messes up return type #12633

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
zamb3zi opened this issue Dec 3, 2016 · 6 comments
Closed

Mapped type function argument messes up return type #12633

zamb3zi opened this issue Dec 3, 2016 · 6 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@zamb3zi
Copy link

zamb3zi commented Dec 3, 2016

TypeScript Version: nightly (2.2.0-dev.20161203)

Code

const foo = <T>(object: T, partial: Partial<T>) => object;
let o = {a: 5, b: 7};
foo(o, {b: 9}); // ok
o = foo(o, {b: 9}) // error: Type '{ b: number; }' is not 
                   // assignable to type '{ a: number; b: number; }'

Expected behavior:

The return type of the function should be {a: number, b: number}, not {b: number}

Actual behavior:

The presence of a Partial<T> argument causes an incorrect return type. Replacing Partial<T> with {} works around the bug.

@zamb3zi
Copy link
Author

zamb3zi commented Dec 3, 2016

I guess T is being inferred from Partial<T> = {b: number} when it should be inferred directly from T = {a: number, b:number}.

@zamb3zi
Copy link
Author

zamb3zi commented Dec 3, 2016

I can partly work around the issue using a second type argument but I'm still blocked from creating a properly typed version of Object.assign:

const assign = <T, P extends T>(target: T, ...sources: Array<Partial<P>>): T =>
    Object.assign(target, ...sources);
let o = {a: 5, b: 7};
assign(o, {a: 9}); // ok
assign(o, {a: 9}, {b: 8}); // error: The type argument for type parameter 'P' 
                          // cannot be inferred from the usage. Consider 
                          // specifying the type arguments explicitly.

Breaking it into separate functions works however:

const assign = <T>(target: T) => (...sources: Array<Partial<T>>): T =>
    Object.assign(target, ...sources);
let o = {a: 5, b: 7};
assign(o)({a: 9});         // ok
assign(o)({a: 9}, {b: 8}); // ok

@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Dec 3, 2016
@zamb3zi
Copy link
Author

zamb3zi commented Dec 3, 2016

This version does seem to mostly work however:

const assign = <T, P>(target: T, ...sources: Array<Partial<T&P>>): T =>
    Object.assign(target, ...sources);
let o = {a: 5, b: 7};
o = assign(o, {a: 6});                // ok
assign(o, {a: 9}, {b: 8}, {});        // ok
assign(o, {a: 9}, {b: false});        // error
assign(o, {a: 9}, {b: 8}, {x: true}); // error
assign(o, {a: 9}, {b: 8}, 'no');      // ok - why? produces {0: "n", 1: "o", a: 9, b: 8}

Edit: This issue of accepting strings (and other primitives) is tracked in #7485.

@ahejlsberg
Copy link
Member

ahejlsberg commented Dec 3, 2016

Ideally we'd have a way of indicating that a particular parameter position should not be an inference location (a request that has come up before). That is effectively what happens when you say T & P. Not sure what that would look like though.

Alternatively, we could say that inferences made to mapped types are classified as secondary and would take effect only if no primary inferences are made. We already have the infrastructure in place to do this kind of classification. The downside would be that in cases where we make inferences to say a T and a Readonly<T>, we would ignore the latter inferences even if they were better.

@mhegazy @DanielRosenwasser Opinons?

@ahejlsberg
Copy link
Member

I'm thinking that classifying inferences to mapped types as secondary is the right thing to do. My reasoning is that such inferences are (a) more speculative than direct inferences and (b) "lossy" because they drop private and protected properties and call and construct signatures. I will put up a PR.

@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Dec 3, 2016
@DanielRosenwasser
Copy link
Member

@ahejlsberg I think making mapped type inferences inferior to those of flat types (like in the PR) is probably the better move. We can see how much of an issue this is if it comes up often and circle back.

@mhegazy mhegazy added this to the TypeScript 2.1.3 milestone Dec 5, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants