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

[@types/react] cannot setState with dynamic key name type-safe #26635

Closed
4 tasks done
peat-psuwit opened this issue Jun 18, 2018 · 13 comments
Closed
4 tasks done

[@types/react] cannot setState with dynamic key name type-safe #26635

peat-psuwit opened this issue Jun 18, 2018 · 13 comments

Comments

@peat-psuwit
Copy link

If you know how to fix the issue, make a pull request instead.

If you do not mention the authors the issue will be ignored.

I cannot call setState with an object being created from computed property name with type-safety:

type State = {
  username: string,
  password: string
};

type StateKeys = keyof State;

class A extends React.Component<{}, State> {
  dynSetState(key: StateKeys, value: string) {
    this.setState({
      [key]: value // Error here. Pretty sure key is in StateKeys
    });
  }
}

I do aware of #18365, and the workaround in #18365 (comment) . However, when using the workaround, Typescript doesn't error out when it should:

  dynLooselySetState(key: string, value: string) {
    this.setState(prevState => ({
      ...prevState,
      [key]: value // No error here, but can't ensure that key is in StateKeys
    }));
  }
@ferdaber
Copy link
Contributor

ferdaber commented Jun 25, 2018

This is a limitation of the compiler itself, inferred types via computed property keys do not support union types, it only supports string, number, symbol or a single literal of those 3, if it detects that it's a union of any of those it will just coerce it into the general string type. The workaround here would be to coerce the object:

dynSetState(key: StateKeys, value: string) {
  this.setState({
    [key]: value
  } as Pick<State, keyof State>)
}

This will still give an error appropriately if the value is not in the set of possible property value types, but will not give you an error if the keys are not within the set of possible property keys.

@Hotell
Copy link
Contributor

Hotell commented Jun 26, 2018

exactly what @ferdaber although IMHO casting stuff like this is not very "good pattern", overall you should prefer updating state via callback pattern which again adheres to best practices, like extracting that callback outside the class to pure easy to test function :)

Good:

class C extends Component<{}, State> {
  updateState(key: StateKeys, value: string) {
    this.setState((prevState) => ({
      ...prevState,
      [key]: value,
    }));
  }
}

Better:

const updateState = <T extends string>(key: keyof State, value: T) => (
  prevState: State
): State => ({
  ...prevState,
  [key]: value
})

class C extends Component<{}, State> {
  doSomething(){
    this.setState(updateState('password','123124'))
  }
}

@jardakotesovec
Copy link

I have not found related issue about this limitation in typescript repo.. Any chance you know if this has been discussed (and where) in typescript repo? Basically wondering if there are some plans to address this in future typescript versions.
Thanks!

@ferdaber
Copy link
Contributor

Here is the discussion, the design notes from the TS team, and an attempt at fixing it (which I believe was retracted for a future version):
microsoft/TypeScript#13948
microsoft/TypeScript#18155
microsoft/TypeScript#21070

@peat-psuwit
Copy link
Author

Indeed, what I'm trying to accomplish is to do something like this: https://reactjs.org/docs/forms.html#handling-multiple-inputs
Which will make it easier to deal with a form with multiple inputs. I may still have to make sure that the "name" attribute is what we expect, but after that type-safety should works.

@joeytwiddle
Copy link

I had to add as unknown to @ferdaber's solution:

  this.setState({
    [key]: value
  } as unknown as Pick<State, keyof State>)

That warned if key was a boolean, but not if it was a number!

So I went for this shorter solution:

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

This warns if key is a boolean or a number.

@neomib
Copy link

neomib commented Jul 23, 2019

Why is #26635 (comment) working? I mean the key is still a union type?

@damir-manapov
Copy link

damir-manapov commented Feb 25, 2020

May be this will help you

type IAction = {
  [P in keyof IAppSettings]?: IAppSettings[P];
};

function reducer(state: IAppSettings, action: IAction) {
  return {
    ...state,
    ...action,
  };
}

@jinggk
Copy link

jinggk commented Apr 16, 2020

Is there any progress in this issue?

@n3rd253
Copy link

n3rd253 commented May 23, 2020

I was able to get this working

handleTextChange(name: keyof State, value: any) {
    this.setState({
        ...this.state,
        [name]: value
    });
}

@arinwt
Copy link

arinwt commented Jun 18, 2020

For others looking at this and trying to apply it to their own state types, note that the type of value in the original post and responses is only string because all property types in State are string. It might be better to avoid using any or additional casting when other types are involved, and instead restricting the types such that the type of value corresponds to its type matching the key type.

interface State {
  name: string;
  age: number;
}
type StateKeys = keyof State;
function dynSetState<K extends StateKeys>(key: K, value: State[K]) {
  this.setState({ [key]: value }); // fails; if only this worked...
  this.setState({ [key]: value } as Pick<State, K>); // clean cast works
  this.setState((s, _) => ({ ...s, [key]: value })); // avoids cast, but changes js
}
dynSetState("name", "a"); // works as expected
dynSetState("name", 1); // fails as expected
dynSetState("age", "a"); // fails as expected
dynSetState("age", 1); // works as expected

Why is #26635 (comment) working? I mean the key is still a union type?

It works because the type of { ...prevState } will sufficiently match State, basically because we have at least all the properties of state. It might help to understand why it doesn't work without it. The root of the issue is that the type of the object created: { [key: K]: State[K] } is not specific enough; it is { [x: string]: State[K] } which can't cast the key string to a keyof State.

@codingwithmanny
Copy link

codingwithmanny commented Jun 19, 2020

Just adding to @arinwt as an another example.

TL;DR: Final Solution At Bottom

Expanded Solution With Console Errors:

type RegisterTypes = {
    email: string;
    password: string;
}

// ...

const [state, setState] = useState<RegisterTypes>({
    email: "",
    password: "",
});

// ...

const onChangeInput = (key: keyof RegisterTypes) => (event: React.ChangeEvent<HTMLInputElement>) => {
    setState({
        [key]: event.target.value
    } as Pick<RegisterTypes, typeof key>);
};

// ...

<input type="email" onChange={onChangeInput('email')} value={state.email} />

Although this gives me the following error in the console:

Warning: A component is changing a controlled input of type password to be uncontrolled. Input elements should not switch from controlled to uncontrolled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component.

Alternative Solution (Cheating) With Console Errors:

Also gives the warning too.

type RegisterTypes = {
    [key: string]: string;
}

// ...

const onChangeInput = (key: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
    setState({
        [key]: event.target.value
    });
};

Final Solution No Errors:

type RegisterTypes = {
    email: string;
    password: string;
}

// ...

const onChangeInput = (event: React.ChangeEvent<HTMLInputElement>) => {
    const newState = { ...state };
    newState[event.target.name as keyof RegisterTypes] = event.target.value;
    setState(newState);
};

// ...

<input name="email" type="email" onChange={onChangeInput} value={state.email} />

@orta
Copy link
Collaborator

orta commented Jun 7, 2021

Hi thread, we're moving DefinitelyTyped to use GitHub Discussions for conversations the @types modules in DefinitelyTyped.

To help with the transition, we're closing all issues which haven't had activity in the last 6 months, which includes this issue. If you think closing this issue is a mistake, please pop into the TypeScript Community Discord and mention the issue in the definitely-typed channel.

@orta orta closed this as completed Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests