Partial does not solve React.setState

[AlexGalays]

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

type State = { veryImportant: number }

// This compiles fine with all the mandatory flags (strictNullChecks, etc)
// if setState uses Partial<State>
// This is a huge invariant violation and can happen very easily for instance
// by setting `veryImportant` to a nullable variable.
this.setState({ veryImportant: undefined })

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 1) Either do not declare a property I have 2) Or if it does, must have the same exact type (not nullable)

By the way, I don't even use React, but this is a fairly common and useful behavior to have.

[ahejlsberg]

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

[AlexGalays]

Looks good! Thanks.

PS: You don't happen to have something like that, but for a recursive structure do you? :)

[ericanderson]

@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]

@ericanderson question: in your opinion, should setState({ foo: undefined }) be an error, or not?

[ericanderson]

Ive been kicking it around in my head. If foo is allowed to be undefined (either because its optional or explicitly allowed), then that should not be an error for setState(), ignoring how TS is implemented.

If you implement setState with Partial, it works as intended for this case but breaks because it allows you to set undefined where its not allowed.

[masonk]

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 call setState({ 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 FooState would return "foo?" | "bar", and Pick would become a perfect expression of the actual type of setState.

(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]

@masonk if you define your state as type TState = { foo: string | undefined } you can still use it as you wish with Pick.

But if we use Partial and you have type State = { foo: string }, then it will not be an error to have setState({foo: undefined}).

[ericanderson]

@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 assign undefined does not get lost.

[masonk]

Sure, 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]

@masonk Correct. I think we could say that there is currently a bug in TypeScript that the undefined get lost in the case of Pick

[ericanderson]

@masonk 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 B implies that there is always a param called foo available, think Object.keys(b). A does not make that guarantee.

[RyanCavanaugh]

I 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 value undefined". It ought to be possible to express this difference to indicate your intent.

[ericanderson]

@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 for C, then I agree that everything is working as it should be.

[AlexGalays]

@ericanderson I think the playground uses strictNullChecks: false

[ericanderson]

@RyanCavanaugh If setState(c, { foo: undefined}); // OK doesn't work with strictNullChecks: true, do you agree that there is a bug?

[masonk]

Ok, those examples from @ericanderson and @RyanCavanaugh are helpful. I now see that there are clear differences between "no key" and "key: undefined".

Damnit, you know what this means?

It means almost all of the interfaces in all of the code I've ever written are maltyped.

Pretty much wherever I have said, { foo?: T }, I meant to say { foo?: T | undefined }. Because {} is assignable to { foo?: T }, I have to treat foo as T | undefined at runtime. With strictNullChecks I'm actually required to. Since I'm already doing that, I usually - read almost always - want to allow myself to assign undefined to foo.

I can't think of a single place in my code where I've made an optional property but wish for my compiler to insist that if the property exists then it must not be undefined. But that's my problem, not TS's problem. TS is just following JS here, and JS clearly makes a distinction.

With that in mind, I now am leaning towards Pick as the correct expression of setState, and I just need to update all of my interfaces.

[ericanderson]

Okay, I found my repro, it fails with strictNullChecks true or false., this feels like it should work @RyanCavanaugh:

interface C {
  foo?: string | undefined;
}
let c: C = {}; // OK
let c2: C = {
    foo: undefined,
} // OK

function setState<T, K extends keyof T>(obj: T, state: Pick<T, K>) {
    for (let k in state) {
        obj[k] = state[k];
    }
}

setState(c, c2); // !OK =  ERR Argument of type 'C' is not assignable to parameter of type 'Pick<C, "foo">'.

[ericanderson]

Nevermind. I withdraw my complaint. In my above example, there is no way for TS to know that c2.foo, in general, was given a value or was left off. Thus if it promises that it returned a Pick<C, "foo">, then foo MUST be defined, which we can't guarantee. While this isn't my ideal, it seems to be correct, and to go back to the source of this (https://github.com/DefinitelyTyped/DefinitelyTyped/pull/13155), Pick is the right choice and the side effect of State objects having no optionals is the most rational thing.

Thanks again @ahejlsberg & @RyanCavanaugh

[masonk]

I'm confused again. How could that possibly be right?

It shouldn't matter to TS whether "c2.foo, in general, was given a value or was left off", because we've said that both are allowed. Pick<C> is a supertype of C. Anything of type C should be assignable to something of type Pick<C>.

[ericanderson]

C cannot be assigned to Pick<C, "foo"> because later when you use the Pick version it MUST have a param of foo that is on the object. C isn't guaranteed to have that

[masonk]

Yes, but how could that possibly be the correct behavior?

keyof C isn't "foo", it's "foo"?, only the type system doesn't know about that yet.

[AlexGalays]

Yeah, that can't be right. You need to be able to reset some state, for instance the id of a timeout when it's over/cancelled should be put back to undefined.

Since a React State is encapsulated, it's not overly helpful to make some of its keys optional, but just removing the ? still doesn't make your example compile.

  interface C {
    foo: string | undefined;
  }
  let c: C = {}; // OK
  let c2: C = {
      foo: undefined,
  } // OK

  function setState<T, K extends keyof T>(obj: T, state: Pick<T, K>) {
      for (let k in state) {
          obj[k] = state[k];
      }
  }

  setState(c, c2);
// error TS2322: Type '{}' is not assignable to type 'C'.
 // Property 'foo' is missing in type '{}'.

Back to square 1! (and that's just the shallow merge :D)

[masonk]

@AlexGalays

  let c: C = {}; // OK

Does that actually work? @RyanCavanaugh's example above that didn't work.

[RyanCavanaugh]

Just to be clear, my comment above was normative not descriptive. Today we considered { x?: number } to be the same as { x: number | undefined } for the purposes of reading x. But we are thinking of changing this to more explicitly model "present but undefined" and "missing"

[ericanderson]

If we are considering {x?: number} to be the same as {x: number | undefined} currently, then currently there is a bug, given the example I pasted above, no?

[AlexGalays]

@masonk Hugh, you're right, the error is at this line rather than the last. But setState(c, {}); works; so that's fine ?

[masonk]

In the playground right now, here is what actually happens. Unexpected* things happens when I pick optional keys.

interface A {
    foo?: string;
}
interface B {
    foo: string | undefined;
}
interface C {
    foo?: string | undefined;
}

let a: A = {};
let a2: A = { foo: "ok" }
let a3: A = { foo: undefined }; // (Expected, but in R.C.'s normative world this might change)

let b: B = { }; // (As expected) Error: Property "foo" is missing. And this is  the behavior in  R.C.'s normative world 
let b2: B = { foo: "ok" };
let b3: B = { foo: undefined };

let c: C = {}; 
let c2: C = { foo: "ok" }
let c3: C = { foo: undefined }

function setState<T, K extends keyof T>(obj: T, state: Pick<T, K>) {
    for (let k in state) {
        obj[k] = state[k];
    }
}
setState(a, {}); // Ok (As expected)
setState(a, a2); // (Unexpected) Error: foo is optional in type 'A' but required in type Pick<A, "foo">
setState(a, a3); // (Unexpected) Error: Argument of type 'A' is not assignable to parameter of type Pick<A, "foo">

setState(b, {}); // Ok (As expected; I have picked 0 keys from B)
setState(b, b2); // Ok (As expected)
setState(b, b3); // Ok (As expected) 

setState(c, {}); // Ok
setState(c, c2); // (Unexpected) Error: foo is optional in type 'C' but required in type Pick<C, "foo">
setState(c, c3); // (Unexpected) Error: Argument of type 'C' is not assignable to parameter of type Pick<C, "foo">

• To me.

In general my contention is that Pick should express a supertype of T. The reason it doesn't right now is because keyof narrows the type by dropping the optionality.

[ahejlsberg]

We've been discussing this and have come to the consensus that Pick<T, K> should copy the modifiers from T which it currently doesn't do. More specifically, for a mapped type { [P in keyof T]: X } we currently copy the modifiers from T, and we want to do the same for a mapped type { [P in K]: X } where K is a type parameter with the constraint K extends keyof T.

[ericanderson]

Thanks @ahejlsbeeg

Will this be in 2.2 or the next 2.1

[masonk]

You guys are the best. Thank you. (By the way, mapped types are an amazing feature; I love them. In fact, 2.1 is an amazing release all around. Ya'll are killing it over there).

[aluanhaddad]

I 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!

[zpdDG4gta8XKpMCd]

sure thing mapped types got some love, let's not skip the legs day tho

[kimamula]

I found another problem with using Pick for typing setState, 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; }'.

[kimamula]

I think if K1 is assignable to K2, then Pick<T, K1> should be assignable to Pick<T, K2>. Given that, the above problem would be resolved.

[AlexGalays]

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]

The 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.

[kimamula]

@ericanderson Ah, yes, you are right.

I was misunderstanding Pick and this comment is incorrect.

I think if K1 is assignable to K2, then Pick<T, K1> should be assignable to Pick<T, K2>.

[kimamula]

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.

[lukewlms]

I don't totally understand the @ahejlsberg solution - we're supposed to redefine setState on every component to get TS to settle down about setState with a partial object?

[grantila]

I agree with @lukecwilliams's not getting how this is solved, especially given the title of this issue. Partial still won't work with setState, since setState uses Pick. Please reopen @ahejlsberg or describe how Partial< T > is compatible with Pick< T, ... >, or conclude that setState should 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 consider foo to 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 tsc happy, à 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.

[coding2012]

I 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 handleCheck2

src/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]

I think you're types could be improved. Specifically to prevent the problem, change the type of name in your functions to keyof ExportProperties. I bet that solves your problem (and also means you dont have to update that function everytime you add something to ExportProperties).

Unrelated but tip, if you use => notation for your methods, you dont have to manually bind them, they are bound for you.