Repository navigation
Partial does not solve React.setState #12793
Description
Activity
I believe this is what you're looking for:
function setState<T, K extends keyof T>(obj: T, state: Pick<T, K>) { for (let k in state) { obj[k] = state[k]; } } interface Foo { a: string; b: number; } let foo: Foo = { a: "hello", b: 42 }; setState(foo, { a: "test", b: 43 }) setState(foo, { a: "hi" }); setState(foo, { b: 27 }); setState(foo, { }); setState(foo, { a: undefined }); // Error setState(foo, { c: true }); // Error
Reacted by Ryan Cavanaugh, Tiago Moraes, Eric Anderson and Soichiro Miki- addedWorking as IntendedThe behavior described is the intended behavior; this is not a bugThe behavior described is the intended behavior; this is not a bug
on Dec 9, 2016 Looks good! Thanks.
PS: You don't happen to have something like that, but for a recursive structure do you? :)
ericanderson commented
on Dec 9, 2016 ContributorMore actionsAnders Hejlsberg (@ahejlsberg) Thanks for the post. However, the following situation then creates an issue:
interface FooState { bar: string; foo?: string; } const defaultFooState: FooState = { bar: "Hi", foo: undefined, }; class Foo extends React.Component<{}, FooState> { public doStuff() { this.setState(defaultFooState); } }
And updating setState as follows:
setState<K extends keyof S>(f: (prevState: S, props: P) => Pick<S, K>, callback?: () => any): void; setState<K extends keyof S>(state: Pick<S, K>, callback?: () => any): void;
You get the error:
severity: 'Error' message: 'Argument of type 'FooState' is not assignable to parameter of type 'Pick<FooState, "bar" | "foo">'. Property 'foo' is optional in type 'FooState' but required in type 'Pick<FooState, "bar" | "foo">'.' at: '37,19' source: 'ts'Updating the state to explicitly allow the optional to be undefined also produces the same error.
interface FooState { bar: string; foo?: string | undefined; }RyanCavanaugh commented
on Dec 9, 2016 MemberMore actionsEric Anderson (@ericanderson) question: in your opinion, should
setState({ foo: undefined })be an error, or not?ericanderson commented
on Dec 9, 2016 ContributorMore actionsIve been kicking it around in my head. If
foois allowed to beundefined(either because its optional or explicitly allowed), then that should not be an error forsetState(), ignoring how TS is implemented.If you implement
setStatewithPartial, it works as intended for this case but breaks because it allows you to set undefined where its not allowed.interface TState = { foo?: string; } let state: TState = { foo: "initial" };
Now I want state to become
{ foo: undefined }, how am I to do that if I can't callsetState({ foo: undefined })?I prefer the Partial to the Pick in this case. Partial allows some invalid behavior, but Pick prohibits some valid expressions.
If index types had the concept of optionality, then
keyof FooStatewould return"foo?" | "bar", and Pick would become a perfect expression of the actual type ofsetState.(Then I think for full completeness, if indexed types gain the concept of optionality they should also gain the concept of requiredness, so that we could undo optionality.
type Required<T> = { [K in keyof T]!: T[K] })
ericanderson commented
on Dec 9, 2016 ContributorMore actionsmasonk if you define your state as
type TState = { foo: string | undefined }you can still use it as you wish withPick.But if we use
Partialand you havetype State = { foo: string }, then it will not be an error to havesetState({foo: undefined}).ericanderson commented
on Dec 9, 2016 ContributorMore actionsRyan Cavanaugh (@RyanCavanaugh) If there is a bug here, I would suggest that any parameter who is an optional is also an implicit "T | undefined" so that when you convert it in something like
Pick, the ability to assignundefineddoes not get lost.Reacted by lucasraySure, but the idiomatic way to say
{ foo: string | undefined }is{ foo?: string }. As I understand TypeScript, those two declarations ought to behave identically throughout TypeScript.ericanderson commented
on Dec 9, 2016 ContributorMore actionsmasonk Correct. I think we could say that there is currently a bug in TypeScript that the undefined get lost in the case of
Pickericanderson commented
on Dec 9, 2016 ContributorMore actionsmasonk Also do note that they arent always assignable.
type A = { foo?: string }; type B = { foo: string | undefined}; let a: A = { foo: "bar" } let b: B = { foo: "bar" } a = b; // okay b = a; // not okay
Update: the definition of
Bimplies that there is always a param calledfooavailable, thinkObject.keys(b).Adoes not make that guarantee.RyanCavanaugh commented
on Dec 9, 2016 MemberMore actionsI think the problem is that the symmetry between
{ foo: string | undefined }and{ foo?: string }is a lie -- spreading in{}to an object has different effects than spreading in{ foo: undefined }.In an ideal world there would be a difference
interface A { foo?: string; } interface B { foo: string | undefined; } interface C { foo?: string | undefined; } let a: A = { }; // OK let b: B = { }; // Error, missing 'foo' let c: C = { }; // OK a.foo = undefined; // Error, undefined is not string b.foo = undefined; // OK c.foo = undefined; // OK setState(a, { }); // OK setState(a, { foo: undefined }); // Error setState(b, { }); // Error, foo missing setState(b, { foo: undefined}); // OK setState(c, { }); // OK setState(c, { foo: undefined}); // OK
We've sort of swept this under the rug, but with Object spread and
Object.keys, there really is a difference between "missing" and "present with valueundefined". It ought to be possible to express this difference to indicate your intent.Reacted by Aluan Haddadericanderson commented
on Dec 9, 2016 ContributorMore actionsRyan Cavanaugh (@RyanCavanaugh) I agree with you, however the example for
setState(c, { foo: undefined});was not working for me on my other laptop (while it does work on the playground right now). If I was imaging the issue forC, then I agree that everything is working as it should be.21 remaining items
aluanhaddad commented
on Dec 11, 2016 ContributorMore actionsI just want to remark that the rapidity of this feedback loop is awesome! An issue comes up, is discussed and gets resolved in less than 2 days, and this is a programming language we are talking about. Great work!
Reacted by Alexandre Galays, Ethan Resnick and Connor Clarksure thing mapped types got some love, let's not skip the legs day tho
Reacted by normalser and Aluan HaddadReacted by Aluan HaddadI found another problem with using
Pickfor typingsetState, which happens when updating state conditionally.declare class Component<P, S> { setState<K extends keyof S>(f: (prevState: S, props: P) => Pick<S, K>, callback?: () => any): void; } declare const component: Component<{}, { foo: number; bar: string; }>; component.setState(() => 0 === 0 ? { foo: 0 } : { bar: '' });
The last line raise an error, which says
error TS2345: Argument of type '() => { foo: number; } | { bar: string; }' is not assignable to parameter of type '(prevState: { foo: number; bar: string; }, props: {}) => Pick<{ foo: number; bar: string; }, "foo...'. Type '{ foo: number; } | { bar: string; }' is not assignable to type 'Pick<{ foo: number; bar: string; }, "foo" | "bar">'. Type '{ foo: number; }' is not assignable to type 'Pick<{ foo: number; bar: string; }, "foo" | "bar">'. Property 'bar' is missing in type '{ foo: number; }'.I think if
K1is assignable toK2, thenPick<T, K1>should be assignable toPick<T, K2>.
Given that, the above problem would be resolved.Yeah, typescript has a lot of issues inferring from function returns. I don't think it's an issue with Pick especially.
It infers that your Pick must have the full set of keys.ericanderson commented
on Dec 29, 2016 ContributorMore actionsThe compiler should probably infer the return value to be 'Pick<T, "foo"> | Pick<T, "bar">' instead. Then it could infer that both of those are assignable to 'Pick<T, K>' I believe.
Eric Anderson (@ericanderson)
Ah, yes, you are right.I was misunderstanding
Pickand this comment is incorrect.I think if K1 is assignable to K2, then Pick<T, K1> should be assignable to Pick<T, K2>.
Well, certainly the issue I described is not specific to
Pick.function f(s: string): void; function f(n: number): void; function f(arg: string | number): void {} f(''); // no error f(0); // no error f(0 === 0 ? '' : 0); // error: Argument of type '"" | 0' is not assignable to parameter of type 'number'. Type '""' is not assignable to type 'number'.
It is likely that although you can define an argument of a function as one of multiple types, it must have a single type per use.
- added a commit that references this issue
on Jun 26, 2017 I don't totally understand the Anders Hejlsberg (@ahejlsberg) solution - we're supposed to redefine setState on every component to get TS to settle down about setState with a partial object?
I agree with @lukecwilliams's not getting how this is solved, especially given the title of this issue.
Partialstill won't work withsetState, sincesetStateusesPick. Please reopen Anders Hejlsberg (@ahejlsberg) or describe howPartial< T >is compatible withPick< T, ... >, or conclude thatsetStateshould have another signature. Anything but leaving it closed when it's still an issue.The following does not work, and is a not-so-uncommon way to partially build up a state somewhere, to then apply it with
setState. Please considerfooto be a lot more complex and not as trivial as this example, where alternative logic would obviously suffice.interface State { a: string; b: number; } class Comp extends React.Component< { }, State > { foo( ) { const state: Partial< State > = { }; if ( !0 ) // obviously apply some useful logic here state.a = "foo"; this.setState( state ); } }
It fails with:
TS2345: Argument of type 'Partial<State>' is not assignable to parameter of type 'Pick<State, "a" | "b">'. Property 'a' is optional in type 'Partial<State>' but required in type 'Pick<State, "a" | "b">'.Yes, one can Hawaii-cast things to make
tschappy, à la:this.setState( state as State );
or, pick your poison:
const state = { } as State;
but this is not a solution. Or is it? I'm personally resentful to blatantly lie about types this way. It's a bad habit.
Reacted by Luke Williams and Josh McDonaldI have another example of some strangeness around the Pick infrastructure as used in React types specifically. Unfortunately I don't fully understand it but this is something I ran into:
typescript version is 2.6.2 and using "@types/react": "^16.0.31"
export type ExportProperties = { hpid: boolean, gender: boolean, age: boolean, diagnosisCode: boolean, medicationCode: boolean, procedureCode: boolean }; export class StudyExportComponent extends React.Component<{ study: Study }, ExportProperties> { constructor(props: { study: Study }) { super(props); this.state = { hpid: true, gender: false, age: false, diagnosisCode: false, medicationCode: false, procedureCode: false }; this.handleCheck = this.handleCheck.bind(this); this.handleCheck2 = this.handleCheck2.bind(this); this.handleCheck3 = this.handleCheck3.bind(this); } handleCheck(name: keyof ExportProperties) { return (event: any) => { let state = {}; state[name] = event.target.checked; this.setState(state); }; } handleCheck2(name: 'hpid' | 'gender' | 'age' | 'diagnosisCode' | 'medicationCode' | 'procedureCode') { return (event: any) => { let boolValue: boolean = event.target.checked; this.setState({ [name]: boolValue }); }; } handleCheck3(name: keyof ExportProperties) { return (event: any) => { let boolValue: boolean = event.target.checked; this.setState({ [name]: boolValue }); }; } // Clipped rendering stuff
With the following errors:
This first one is for handleCheck2src/components/study/study-export.tsx(41,27): error TS2345: Argument of type '{ [x: string]: boolean; }' is not assignable to parameter of type 'ExportProperties | ((prevState: Readonly<ExportProperties>, props: { study: Study; }) => ExportPr...'. Type '{ [x: string]: boolean; }' is not assignable to type 'Pick<ExportProperties, "hpid" | "gender" | "age" | "diagnosisCode" | "medicationCode" | "procedur...'. Property 'hpid' is missing in type '{ [x: string]: boolean; }'. 41 this.setState({ [name]: boolValue }); ~~~~~~~~~~~~~~~~~~~~~And this one for handleCheck3:
src/components/study/study-export.tsx(47,27): error TS2345: Argument of type '{ [x: string]: boolean; }' is not assignable to parameter of type 'ExportProperties | ((prevState: Readonly<ExportProperties>, props: { study: Study; }) => ExportPr...'. Type '{ [x: string]: boolean; }' is not assignable to type 'Pick<ExportProperties, "hpid" | "gender" | "age" | "diagnosisCode" | "medicationCode" | "procedur...'. Property 'hpid' is missing in type '{ [x: string]: boolean; }'. 47 this.setState({ [name]: boolValue }); ~~~~~~~~~~~~~~~~~~~~~My work-around is clearly in handleCheck which does not have any TS errors but as you can see is clearly working around types in this case (at least I think it is).
I also just noticed that this compiles just fine:
handleCheck(name: keyof ExportProperties) { return (event: any) => this.setState({ [name]: event.target.checked } as Pick<ExportProperties, keyof ExportProperties>); }
Now that I look at this a little more maybe that's the appropriate thing to do... And in fact when I change the property type of the function parameter name to something like name: 'catfish' I definitely get errors about casting the Pick<ExportProperties, keyof ExportProperties> which is exactly what I wanted. So I think this last example is how it should be done.
Now just to be clear none of this is failing the most common use-case as shown below:
this.setState({ hpid: true });
Hopefully this post is helpful to others even if not actionable to the Typescript language. Also cheers to the Typescript team and @types/react who have made this typing on top of react so freaking helpful. Everything I do in my tsx html and of course my state and props variables are all fully typed due to this work. It's incredibly useful.
ericanderson commented
on Dec 27, 2017 ContributorMore actionsI think you're types could be improved. Specifically to prevent the problem, change the type of
namein your functions tokeyof ExportProperties. I bet that solves your problem (and also means you dont have to update that function everytime you add something toExportProperties).Unrelated but tip, if you use => notation for your methods, you dont have to manually bind them, they are bound for you.
- locked and limited conversation to collaborators
on Jul 3, 2018
Traditionally in TS, the absence of value or an explicit undefined value was exactly the same thing.
Perhaps it still even makes sense for Partial, but I thought Partial was meant to solve the React setState problem; and I believe it's still not solved today.
TypeScript Version: 2.1.4
Code
Expected behavior:
A non nullable property should not be updatable with null/undefined with strictNullChecks.
We need to be able to say "give me an object that
By the way, I don't even use React, but this is a fairly common and useful behavior to have.