Typescript: Computed property key names should not be widened

Created on 8 Feb 2017  ·  30Comments  ·  Source: microsoft/TypeScript

TypeScript Version: 2.1.5

Code
The latest @types/react (v15.0.6) use Pick<S,K> to correctly type the setState method of React.Components. While this makes it now possible to merge the state of a component instead of replacing it, it also makes it harder to write a dynamic update function that uses computed properties.

import * as React from 'react';

interface Person {
  name: string;
  age: number|undefined;
}

export default class PersonComponent extends React.Component<void, Person> {
  constructor(props:any) {
    super(props);

    this.state = { 
      name: '',
      age: undefined
    };
    this.handleUpdate = this.handleUpdate.bind(this);
  }

  handleUpdate (e:React.SyntheticEvent<HTMLInputElement>) {
    const key = e.currentTarget.name as keyof Person;
    const value = e.currentTarget.value;
    this.setState({ [key]: value }); // <-- Error
  }

  render() {
    return (
      <form>
        <input type="text" name="name" value={this.state.name} onChange={this.handleUpdate} />
        <input type="text" name="age" value={this.state.age} onChange={this.handleUpdate} />
      </form>
    );
  }
}

The above should show an actual use case of the issue, but it can be reduced to:

const key = 'name';
const value = 'Bob';
const o:Pick<Person, 'name'|'age'> = { [key]: value };

which will result in the same error. Link to the TS playground

Expected behavior:
No error, because key is a keyof Person, which will result in the literal type "name" | "age". Both values that are valid keys forstate.

Actual behavior:
The compiler will throw the following error:

[ts] Argument of type '{ [x: string]: string; }' is not assignable 
     to parameter of type 'Pick<Person, "name" | "age">'.
       Property 'name' is missing in type '{ [x: string]: string; }'.

My uninformed guess is that the constant key is (incorrectly) widened to string.

Bug

Most helpful comment

A small note here, that @lucasleong solution introduces a possible runtime bug (and a very subtle one too) in React, since State updates may be asynchronous.

You should never use this.state to calculate the new state using the this.setState({ ... }) syntax! The more correct solution would be using an updater function for the state instead, so we can make sure we always spread the correct up-to-date state into the new state:

private handleChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
    this.setState((prevState) => ({
        ...prevState,
        [e.target.name]: e.target.value
    }));
}

I think there is actually a better method to type this right now using mapped types, which doesn't even change any runtime behavior (as the state spread version does):

private handleChange = <T extends keyof IState>(event: React.ChangeEvent<HTMLInputElement>) => {
  const newState = {
    [event.target.name]: event.target.value,
    // keyNotInState: '42', -> would throw a compile time error
    // numericKeyInState: 'assigning wrong type' -> would throw a compile time error
  };
  this.setState(newState as { [P in T]: IState[P]; });
}

That way the compiler is even able to catch if you are accidentally trying to assign wrong keys explicitly in the newState object, which the spread previous state method would also silently allow (since the whole required state is already in the object, via the spread).

All 30 comments

My uninformed guess is that the constant key is (incorrectly) widened to string.

That is correct.
Note that by declaring the type of o to be Pick<Person, 'name'|'age'> you require that both name and age be present in the value.

Ok, so the above React example can never work, because setState requires explicit input since the function expects a Pick<S, K>? Which would mean that the issue is with the Typings and this is absolutely not the place to ask this question? 🙃

@sebald You might mean something like

let peter: Pick<Partial<Person>, 'name' | 'age'> = {
    [key]: value
};

In any case, I don't think key should widen, so there looks like a bug here.

@DanielRosenwasser thanks for the quick reply! I was wondering if using Partial is better to type setState.

For everyone that has the same issue: As a temporary fix you can write this.setState({ [key]: value });

@sebald

thanks for the quick reply! I was wondering if using Partial is better to type setState.

For everyone that has the same issue: As a temporary fix you can write this.setState({ [key]: value });

Partial wouldn't be ideal either because you could then update all non nullable properties with undefined.

Btw you posted a workaround that looks identical to the original code?

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

with https://github.com/Microsoft/TypeScript/pull/18317 in the simple case with a single literal type works, but a union of literals still does not. we should distribute the union over the computed property.

The workaround found here worked for me

this.setState({ [key as any]: value });

I read it thoroughly but I cannot understand why this issue is still considered a bug. There are two blocks of code in original post. Second one is an abvious error because Pick<Person, 'name'|'age'> expects both name AND age to be present on assigned object. First one is more complex and for me it's still an error by design. Consider this minimal example in TS Playground. I've added comments before the "setState" lines that describe the cases. The third one is the case from opening post's first code example.

However! Look at this example which is similar but with different People interface. This one throws an error, even though, in my opinion, it shouldn't.

@sandersn please make sure the example in https://github.com/Microsoft/TypeScript/issues/20201 works as expected.

Fix is up at #21070

Does not work anymore with 2.9.1:

this.setState({ [key as any]: value })

I have to ts-ignore it now:

// @ts-ignore
this.setState({ [key]: value });

Any better solution with 2.9.1?

@tkrotoff This works for me:

const o = {} as { name, age }
o[key] = value
this.setState(o)

I'm not quite sure if this is (not?) part of the bug, but my scenario looks as follows:

enum EnumDefinition {
    NAME = "name"
}

interface State {
    [EnumDefinition.NAME]: string
    test?: string
}

class ComponentName extends React.Component<Props, State> {

    public render() {
        <input name={EnumDefinition.NAME} onChange={this.handleChange} />
    }

    private handleChange = (event: React.ChangeEvent<HTMLInputElement>): void => {
        this.setState({[event.target.name]: event.target.value});
    }
}

Which ends up with something similar to:
Type '{ [x: string]: string; }' is not assignable to type 'Pick<State, EnumDefinition.NAME | "test". Property 'name' is missing in type '{ [x: string]: string; }'.

Before 2.9 this hack this.setState({[event.target.name as any]: event.target.value}); helped to get rid of errors, but now it doesn't help unfortunately.

Anyone solved this "issue" with 2.9? I have the same problem as @tkrotoff and @oszbart

@AlbertoAbruzzo I solved this by first expanding the current state, using the spread operator. Then apply computed property name to update the state like normal. This method seems to work without giving an error.

For example, using @oszbart's example:

private handleChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
    this.setState({
        ...this.state,
        [e.target.name]: e.target.value
    });
}

This solution has a possible runtime bug, see @timroes solution for a workaround!

Is there any performance issue for @lucasleong solution ? :/

@theapache64 Yes, it adds an extra Object.assign call. I doubt it will be in the top 10 bottlenecks of a react app though :)

A small note here, that @lucasleong solution introduces a possible runtime bug (and a very subtle one too) in React, since State updates may be asynchronous.

You should never use this.state to calculate the new state using the this.setState({ ... }) syntax! The more correct solution would be using an updater function for the state instead, so we can make sure we always spread the correct up-to-date state into the new state:

private handleChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
    this.setState((prevState) => ({
        ...prevState,
        [e.target.name]: e.target.value
    }));
}

I think there is actually a better method to type this right now using mapped types, which doesn't even change any runtime behavior (as the state spread version does):

private handleChange = <T extends keyof IState>(event: React.ChangeEvent<HTMLInputElement>) => {
  const newState = {
    [event.target.name]: event.target.value,
    // keyNotInState: '42', -> would throw a compile time error
    // numericKeyInState: 'assigning wrong type' -> would throw a compile time error
  };
  this.setState(newState as { [P in T]: IState[P]; });
}

That way the compiler is even able to catch if you are accidentally trying to assign wrong keys explicitly in the newState object, which the spread previous state method would also silently allow (since the whole required state is already in the object, via the spread).

I think this may be related. I'm seeing a problem with this in V3.1.1:


import React from 'react';

interface Props<T> {
    data: T;
}

interface State<T> {
    a: number;
    b: number;
    data: T;
}

const f = (): { a: number } | {} => ({});

export class Table<T> extends React.Component<Props<T>, State<T>> {
    constructor(props: Props<T>) {
        super(props);
        this.state = { a: 1, b: 2, data: props.data };
    }

    public update() {
        this.setState(f()); // << error here
    }
}

Error text:


Argument of type '{} | { a: number; }' is not assignable to parameter of type 'State<T> | ((prevState: Readonly<State<T>>, props: Props<T>) => State<T> | Pick<State<T>, "a"> | null) | Pick<State<T>, "a"> | null'.
  Type '{}' is not assignable to type 'State<T> | ((prevState: Readonly<State<T>>, props: Props<T>) => State<T> | Pick<State<T>, "a"> | null) | Pick<State<T>, "a"> | null'.
    Type '{}' is not assignable to type 'Pick<State<T>, "a">'.
      Property 'a' is missing in type '{}'.

I've encountered similar problem...

interface ILoginState {
    username: string;
    password: string;
    message: string;
    isError: boolean;
    isLoaded: boolean;
}

// in a class component
//...
handleChange = (key1: keyof ILoginState) => (e: any) => {
  // Error: property 'password' is missing in type '{ [x: string]: any }'
  this.setState({ [key1]: e.target.value });
};

Problem:

‪type S = "a" | "b";‬

‪interface If {‬
‪    [_ in S]: Type‬
‪}‬

Workaround:

‪```‬ts
‪interface If {‬
‪ a: Type;‬
‪ b: Type;‬
‪}‬

‪type S = keyof If;‬
‪```

See this tweet.

The mini repro now passes for individual string literals, but here's a repro that still fails. Note that key's type is a union.

interface Person {
  name: string;
  second: string|undefined;
}
export function test(key: 'name' | 'second') {
    const value = 'Bob';
    const o:Pick<Person, 'name'|'second'> = { [key]: value };
    return o
}

How about this? (SO)

  this.setState<never>({
    [key]: value
  })

That tells setState that it should expect no properties in particular. And then we pass an extra property, which it doesn't seem to mind.

Edit: Yeah this doesn't check the types, or fix this issue. It's just a quick workaround for anyone who is stuck transpiling, and came here for help.

  this.setState<never>({
    [key]: value
  })

This wouldn't verify that value is of the right type, right?

Confused as to why the following code doesn't work in TypeScript 3.4 (view in TypeScript Playground)

function mapEntity<T extends string | number, V extends unknown>({
  key,
  value,
}: {
  key: T;
  value: V;
}): { [P in T]: V } {
  return { [key]: value };
}

The error is:

  return { [key]: value };
  ^^^^^^^^^^^^^^^^^^^^^^^^
  Type '{ [x: string]: V; }' is not assignable to type '{ [P in T]: V; }'.

Seems to be related to this issue?

That should work

// react.d.ts
import * as react from 'react';

declare module 'react' {
  interface Component<P, S> extends react.Component<P, S> {
    setState<K extends keyof S>(
      state:
        | ((
            prevState: Readonly<S>,
            props: Readonly<P>,
          ) => Partial<Pick<S, K>> | S | null)
        | (Partial<Pick<S, K>> | S | null),
      callback?: () => void,
    ): void;
  }
}

The string literal change was a nice stopgap but definitely falls short. It would be nice to see this work with unique symbol types as well, since you actually must use computed keys.

const Key = Symbol();
type Thing = {} | {
    [Key]: string;
    property: string;
};

declare const thing: Thing;
const test1 = 'property' in thing ? thing['property'] : undefined; // this is fine
const test2 = Key in thing ? thing[Key] : undefined; // this is an error

cross-linking to #21030

Was this page helpful?
0 / 5 - 0 ratings

Related issues

uber5001 picture uber5001  ·  3Comments

fwanicka picture fwanicka  ·  3Comments

DanielRosenwasser picture DanielRosenwasser  ·  3Comments

Antony-Jones picture Antony-Jones  ·  3Comments

seanzer picture seanzer  ·  3Comments