Skip to content
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

Spread with enum index key, invalid members don't trigger errors #32236

Open
osdiab opened this issue Jul 3, 2019 · 5 comments
Open

Spread with enum index key, invalid members don't trigger errors #32236

osdiab opened this issue Jul 3, 2019 · 5 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@osdiab
Copy link

osdiab commented Jul 3, 2019

TypeScript Version: 3.5.1

Search Terms:
object spread enum index missing keys

Code
Playground link has code

Expected behavior:
Missing keys & incorrect value types when I build a nested object using the spread operator with index keys, ought to throw errors for invalid types

Actual behavior:
It seems like the child object can have whatever shape I want, the types aren't being checked at all

Playground Link: This is same as above playground link

Related Issues: I'm not really sure what the root limitation that's causing this is so it's hard for me to find similar issues.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 3, 2019

Simplified

type PropName = "A" | "B";
type MyMappedType = {
    [key in PropName]: string;
};
declare const AB: PropName;
const k1: MyMappedType = {
    A: "",
    B: "",
    // No error
    [AB]: 10
};
const k2: MyMappedType = {
    A: "",
    B: "",
    ...{
      // Also no error
      [AB]: 10
    }
};

const k3 = {
    A: "",
    B: "",
    [AB]: 10
};
// No error
const j: MyMappedType = k3;

@RyanCavanaugh
Copy link
Member

So the problem here is that there's not a good way to describe the type of k3 in the above example - it's technically { A: string; B: number } | { A: number; B: string } but this type algebra is combinatorially explosive in the presence of multiple computed property names, and isn't actually correct if given e.g. { [AB]: "", [AB]: "" }. At least in the indirected case, it's not clear that we have any available fix.

That said, ideally we'd have some way to flag the [AB] property in k1 as an error by realizing it's a contextually typed thing, keeping the literalness of AB around, and then making sure it's assignable to the intersection of the properties it possibly targets.

@ahejlsberg any thoughts?

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jul 3, 2019
@dragomirtitian
Copy link
Contributor

dragomirtitian commented Dec 17, 2019

@RyanCavanaugh This seems to affect not just enums. If the computed property is string it basically gets ignored when typing the result of the spread:

declare let o: { field1: number };
declare let key: string;

o = { ...o, [key]: "value" };  // ok ? why ?
let newO = { ...o, [key]: "value" };  // same type as o ? but why ?

declare let oIndex: { [n: string]: number };
oIndex = { ...oIndex, [key]: "value" } // here we get an error

Playground Link

I really don't think the result of { ...o, [key]: "value" } should be typeof o unless key is a specific key of typeof o. This is fundamentally unsound and surprising.

@osdiab
Copy link
Author

osdiab commented Dec 17, 2019

@dragomirtitian i think technically that’s ok because the resulting object may have other keys but still satisfies the interface that the object type specifies - though it’s also the case that applying that key-value pair may cause an existing key on the object to have an incorrect value type, which is unsound

@danilowanner
Copy link

I am running into the same issue. Any update on this?

It is fairly common to update an object using code similar to this:

type Key = 'a' | 'b' | 'x'
type Obj = {[key in Key]: number}

const x = 'x' as Key

const obj: Obj = {
  a: 4,
  b: 4,
  x: 4,
}

const updatedObj: Obj = {
  ...obj,
  [x]: 'hello', // No error. No type check on the value, which is a string instead of number
}

The same is true for Object.assign:

const updatedObj: Obj = Object.assign({}, obj, { [x]: '4' }) // No error

It seems that { [x]: '4' } gets widened to the type { [x: string]: string }, but arguably something like the following would be more accurate: {a: string} | {b: string} | {x: string}. Even then it is currently still a valid third argument to Object.assign.

This issue may be related to:
#37103
#13948
#38663

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

5 participants