Union in a computed property allows any assignment to property value

[zcabjro]

TypeScript Version: 4.0.0-dev.20200518

Search Terms: computed property, union

Code

type Key = 'a' | 'b';

const index1: Record<Key, string> = { a: '', b: 0 }; // Errors as expected, ok
const index2: Record<Key, string> = { a: '', b: '' };

const b: Key = 'b';

const index3: Record<Key, string> = { ...index2, [b]: 0 }; // Errors as expected, ok

declare var k: Key;

// No errors, allowing anything to be assigned to string
const index4: Record<Key, string> = { ...index2, [k]: [] };
const index5: Record<Key, string> = { a: '', b: '', [k]: 0 };

Expected behavior:
Expected error for invalid assignments in index4 and index5.

Actual behavior:
No error, allowing anything to be assigned to string

Playground Link: here

Related Issues:
#36920: perhaps the computed property is deemed an excess property, though I don't think it should be

[RyanCavanaugh]

@weswigham what's the designed behavior here?

[jcalz]

#13948 + #27144 = 💥 ?

[weswigham]

So, there's two things, first:

declare var str: string;

const x = { a: "", b: "" };

const y = { [str]: 0, ...x };

const z = { ...x, [str]: 0 };

A computed property name with a non-literal name produces an index signature. getSpreadType in the compiler elides the index signature from the output type if either input elides the index signature. So you'll note in the above, no type has an index signature - the types introduced by the computed names simply evaporate. Now... including it is a little awkward, as it can easily produce a type like

{
    [x: string]: number;
    a: string;
    b: string;
}

where the index signature does not actually cover all the members in the type, but that can be fixed.

Second, Record<Key, string> is {a: string, b: string} - when {[x: string]: whatever, a: string, b: string} is assigned to it, the index signature is not considered to be an excess property. This is done for good reason! If the computed name only actually edits the known names, then there aren't any excess properties (and index signatures being as wishy-washy as they are, that's how we prefer it). Since the index signature can't be excess, the desired error must come from property assignability - which, when you look at it, you realize something's missing - the issue is that the computed property doesn't edit the types of the properties it may overwrite!

[nth-commit]

+1 on this, allowed a bad type to silently survive during a refactor for us.

[nth-commit]

Here's my minimal example, similar to OP's:

type Key = 'foo' | 'bar';

// Ex 1. Attempting to assign `unknown` in place of `string`

declare const unknownValue: unknown;

const dict1: Partial<Record<Key, string>> = {
  foo: unknownValue, // Error ✔️
};

// Ex 2. Attempting to assign `unknown` in place of `string`, except this time using a computed property

declare const keyValuePair: {
  key: Key;
  value: unknown;
};

const dict2: Partial<Record<Key, string>> = {
  [keyValuePair.key]: keyValuePair.value, // No Error ❌
};

[RyanCavanaugh]

I think we could get away with raising an error for t = { [k]: v } when k is a literal union type "s1" | "s2" | "s3" ... and v isn't assignable to the union t["s1"] | t["s2"] | t["s3"] | ... (intersection would be more sound, but impractical due to producing never a lot of the time). It'd be a breaking change but it's hard to see how that code wouldn't have an actual bug.

[ChristianIvicevic]

The examples in the previous posts show unexpected behavior where the value is not type-safe. I noticed a similar issue with a dynamic key, although I am not sure whether this is strictly related.

My use-case is working with prisma as an ORM to dynamically update a column where a correct call looks like this:

prisma.table.update({
  data: {
    columnToUpdate: newValue,
  }
});

I wanted to dynamically select the column to update and did Pick legal keys from that data property and noticed the dynamic keys not being type-safe. Here is my minimal example:

type T = {
  one?: 1;
};
type Keys = 'one' | 'two';
const dynamicKey = (): Keys => 'two';
const absurd: T = {
  [dynamicKey()]: 2 // TS silently accepts this, both key AND value
};

[fatcerberus]

intersection would be more sound, but impractical due to producing never a lot of the time

You say that, but #30769 exists. 🚎 (Lots of people did used to trip on that one, but it doesn’t seem like a big stumbling block anymore.)

[LBBO]

Is there any update on this? Could this at least be considered a bug? This has allowed a very preventable bug to exist in our code for ages where a string value is set in an object that may only contain numbers (minimal example below). I realise it would be a breaking change, but maybe it could at least be added behind a compiler option or something.

type OnlyNumberValues = {
    foo: number
    bar: number
}

const whyDoesThisWork = (obj: OnlyNumberValues, key: keyof OnlyNumberValues): OnlyNumberValues => ({
    ...obj,
    [key]: 'definitely not a number'
})