TypeScript: Partial does not solve React.setState

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.

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 3
  • Comments: 42 (25 by maintainers)

Commits related to this issue

Most upvoted comments

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

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

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

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!

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.

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.

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.

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.

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