Skip to content

Generic mapped type assignability rules don't check modifiers type properly #27598

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
mattmccutchen opened this issue Oct 7, 2018 · 11 comments
Closed
Assignees
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@mattmccutchen
Copy link
Contributor

More warmup for #27484.

TypeScript Version: master (85a3475)

Search Terms: generic mapped type assignable assignability relation optional modifiers type

Code

let x: {a?: string} = {};
let y: {a: string | undefined};
y = x;  // Error, as expected

type WeirdPick1<A extends {[P in B]?: unknown}, B extends keyof any> = {[P in B]: A[P]};
type WeirdPickAll1<T> = WeirdPick1<T, keyof T>;
y = (<T>(arg: T): WeirdPickAll1<T> => arg)(x);  // OK; expected error

type WeirdPickAll2<T> = {[P in keyof Required<T>]: T[P]};
y = (<T>(arg: T): WeirdPickAll2<T> => arg)(x);  // OK; expected error

type WeirdPickAll3<T> = {[P in keyof Partial<T>]: T[P]};
y = (<T>(arg: WeirdPickAll3<T>): T => arg)<{a: string}>(x);  // OK; expected error

Expected behavior: All three WeirdPick constructions give errors.

Actual behavior: None of the WeirdPick constructions give errors.

Playground Link: link

Related Issues: none found

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Oct 9, 2018
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.2 milestone Oct 9, 2018
@ahejlsberg
Copy link
Member

In homomorphic mapped types of one of the forms { [P in keyof T]: XXX } or { [P in K]: XXX }, where K has a constraint of the form keyof T, we preserve property modifiers (i.e. readonly and ?) as they exist in T. In all other cases, the only property modifiers that will be present are those specified in the mapped type itself. See #12563 for more info.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs Investigation This issue needs a team member to investigate its status. labels Oct 9, 2018
@mattmccutchen
Copy link
Contributor Author

@ahejlsberg I understand. The problem is that the assignability rules are inconsistent with actual mapped type instantiation in the way they check for the special forms { [P in keyof T]: XXX } and { [P in K]: XXX } (where K is constrained by keyof T), hence the examples of unsoundness in my original comment. Please reopen. I have a draft fix for this but haven't finalized the semantics and test cases in order to submit a PR.

@n9
Copy link

n9 commented Oct 24, 2018

I have hit a similar issue. However I am not sure whether it is the same problem or not. If not, I will open another issue.

// my code

type FieldKind = "numeric" | "text"

interface FieldMeta {
    kind: FieldKind 
}

type FieldsMeta<T> = { [K in keyof T]-?: FieldMeta }

type Compoment = "aComponent"

type FieldComponents<T> = { [K in keyof T]-?: Compoment }

function createComponentsFromMeta<T>(meta: FieldsMeta<T>): FieldComponents<T> {
    return mapValues(meta, (value, key) => "aComponent" as Compoment) // <-- type error here
}

// external lib

declare type ObjectKey = string | number | symbol

declare type ObjectMap<K extends ObjectKey, V> = { [P in K]: V };
declare type MapObject<K, I, O> = (value: I, key: K) => O;

 // something like lodash.mapValues
declare function mapValues<K extends ObjectKey, I, O>(obj: ObjectMap<K, I>, map: MapObject<K, I, O>): ObjectMap<K, O>

This will fail with 'ObjectMap<keyof T, "aComponent">' is not assignable to type 'FieldComponents'.
FieldsMeta uses -? in order to require metadata also for optional fields (the same is for FieldComponents), for instance:

interface Foo {
    a: number
    b?: string
}

const fooMeta: FieldsMeta<Foo> = {
    a: { kind: "numeric" }, 
    b: { kind: "text" } 
}

@mattmccutchen
Copy link
Contributor Author

@n9 Your case is different from the cases in my original report (your case involves assignability between two generic mapped types, while mine involve assignability between a generic mapped type and another type), but it still falls under the summary of "Generic mapped type assignability rules don't check modifiers type properly", so I am happy to consider it included in this issue. Now we just hope for the issue to be unmarked as "Working as intended"!

A workaround is to change the definition of FieldComponents to:

type FieldComponents<T> = Record<keyof T, Compoment>;

Then instead of using -?, which confuses the current assignability rule, you are using a mapped type that doesn't have a modifiers type (since the keyof constraint is not part of the original definition of the mapped type in Record), so all properties will be required.

@n9
Copy link

n9 commented Oct 25, 2018

@mattmccutchen Great, thanks for the workaround!

To be honest, I've just monkey-copied your pattern. But still I do not understand why you pattern works and does not require -?? Should I understand that keyof somehow lost information, that it contains ? members? What causes that the ? is lost? Passing keyof as generic argument?

@mattmccutchen
Copy link
Contributor Author

@n9 Roughly. This is one of TypeScript's many underdocumented rules that tries (and sometimes fails) to do what you want. The decision of the "modifiers type" (if any) from which modifiers will be copied is based on the original definition of the mapped type, before any type parameters are instantiated. When you write type FieldComponents<T> = { [K in keyof T]-?: Compoment }, the keyof T appears in the original definition, so T is saved as the expression for the modifiers type and then gets instantiated with the type argument you passed for T. When you write type FieldComponents<T> = Record<keyof T, Compoment>, the original definition of Record is type Record<K extends keyof any, T> = {[P in K]: T}, and the constraint K is a type parameter with constraint keyof any, so the modifiers type is any (or maybe there's no modifiers type at all, but either way, there are no modifiers to inherit). This decision is made before K is instantiated as keyof T.

@n9
Copy link

n9 commented Oct 25, 2018

@mattmccutchen Thank you for your explanation. These modifiers look really great at first sight. However it seems to me now, that they are rather fragile. So there is difference between Working as Intended and Working as Expected.

@ahejlsberg, @RyanCavanaugh In case this is Working as Intended, there should be another label Not Working as Expected 😊

@mattmccutchen
Copy link
Contributor Author

mattmccutchen commented Oct 25, 2018

I agree that the determination of whether a mapped type is homomorphic is fragile and surprising, but I wouldn't expect it to be changed at this point. But that's not what this issue is about. This issue is about the assignability rules being inconsistent with the way homomorphic mapped types are determined. ahejlsberg's comment accompanying the "working as intended" label does not appear to address the cases in my original report, so I believe he just didn't read the issue carefully enough.

@ahejlsberg
Copy link
Member

@mattmccutchen Am I correct that the issue you're referring to is this?

type Pick1<A, B extends keyof any> = {[P in keyof A & B]: A[P]};
type Pick2<A, B extends keyof any> = {[P in keyof A & B]-?: A[P]};

function foo<T>(x: T, y: Pick1<T, keyof T>, z: Pick2<T, keyof T>) {
    y = x;  // Ok
    z = x;  // Error
}

In other words, since Pick1 is not a homomorphic mapped type it should behave like Pick2.

@mattmccutchen
Copy link
Contributor Author

@ahejlsberg There are lots of cases in which the existing assignability rules for generic mapped types are inconsistent with the way generic mapped types are instantiated, and I was hoping that this issue would cover all of the inconsistencies we can find. (The issue by itself is low priority unless/until realistic code is affected by the problems; I filed it because I thought if we planned to redo the assignability rules anyway for #27484, we should try to get all of these cases fixed at the same time.)

Your example is equivalent to the WeirdPick1 case from the original report, but we now have three more examples (two more in my initial comment and one from n9), all of which (AFAICT) affect different compiler code paths.

@DanielRosenwasser DanielRosenwasser removed this from the TypeScript 3.2 milestone Oct 30, 2018
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

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

6 participants