pmndrs / zustand

🐻 Bear necessities for state management in React
https://zustand-demo.pmnd.rs/
MIT License
48.39k stars 1.5k forks source link

[Question] [TypeScript] Best way to decouple set() function #628

Closed magiccookie closed 2 years ago

magiccookie commented 3 years ago

Hello! I wish to be able to dispatch a function instead of using setters and getters. But how is it better to deal with its type?

interface MyStore {
  bears: number;
  honey: number;
  dispatch: (fn: HandlerFn | Partial<MyStore>) => void;
}

// Expected dispatch handler type to be something like:
type HandlerFn = (state: MyStore) => Partial<MyStore>;

export const useStore = create<MyStore>((set) => ({
  bears: 1,
  honey: 3,
  dispatch: (fn: HandlerFn | Partial<MyStore>) => set(fn)
}));

export const incBears = (state: MyStore) => ({
  bears: state.bears + 1,
  honey: state.honey + 3
});

export const incJustHoney = (state: MyStore) => ({
  honey: state.honey + 1
});

export const lotsOfBears = { bears: 999 };

example usage <button onClick={() => dispatch(incBears)}> .

I expect my handler type to be as HandlerFn to return Partial store (or just Partial store in case of an object). However Partial doesn't match with the set interface (also I tried to compose anything nice out of zustand types)

The error is:

(parameter) fn: HandlerFn
Argument of type 'HandlerFn' is not assignable to parameter of type 'PartialState<MyStore, keyof MyStore, keyof MyStore, keyof MyStore, keyof MyStore>'.
  Type 'HandlerFn' is not assignable to type '(state: MyStore) => MyStore | Pick<MyStore, keyof MyStore>'.
    Type 'Partial<MyStore>' is not assignable to type 'MyStore | Pick<MyStore, keyof MyStore>'.
      Type 'Partial<MyStore>' is not assignable to type 'Pick<MyStore, keyof MyStore>'.
        Types of property 'bears' are incompatible.
          Type 'number | undefined' is not assignable to type 'number'.
            Type 'undefined' is not assignable to type 'number'.ts(2345)

Basically, if I just want to proxy the set function, what would be the best way to annotate handler function?

I think I could have specified the handler types as for example (state: MyStore) => Pick<MyStore, "bears", "honey">, however, I don't always know which keys my function return

See full example here https://codesandbox.io/s/hopeful-cohen-xy33j?file=/src/store.ts

dai-shi commented 3 years ago

(pinning @TkDodo to share this use case.)

I'm not sure if there's a good way. I would probably give up and do set(fn as any). 😎

If anyone has suggestions, please jump in.


As a side note, assuming we don't need to reuse incBears for multiple stores. Here's what I would do:

interface MyStore {
  bears: number;
  honey: number;
}

export const useStore = create<MyStore>(() => ({
  bears: 1,
  honey: 3,
}));

export const dispatchIncBears = () => useStore.setState((state) => ({
  bears: state.bears + 1,
  honey: state.honey + 3
}));

export const dispatchIncJustHoney = () => useStore.setState((state) => ({
  honey: state.honey + 1
}));

export const dispatchLotsOfBears = () => useStore.setState({ bears: 999 });

This is the pattern how we often do in valtio and it's good for code splitting.

magiccookie commented 3 years ago

@dai-shi Thanks a lot for sharing

TkDodo commented 3 years ago

I expect my handler type to be as HandlerFn to return Partial

The problem with the Partial type is that it doesn't differentiate between a key not being present or a key being present, but having an undefined value.

so if incBears were to return a Partial<Store>, you could do:

export const incBears = (state: MyStore) => ({
  bears: state.bears + 1,
  honey: undefined // 🚨
});

and TypeScript would be totally fine with that. Since the set function internally just spreads whatever you return over the current state, you would wind up with honey: undefined in your store, even though you only allow numbers.

This is especially troublesome if you have a property that can be undefined, and later refactor it to be required. In those situations, TypeScript wouldn't show you all the places where you set that value to undefined.

The type we have in zustand tries to infer the type from the keys of the returned object, and it's inspired by how class based setState in react works.

I'm not sure if there's a good way. I would probably give up and do set(fn as any). 😎

I also couldn't find a better way, sorry.

dai-shi commented 3 years ago

@devanshj This is another type issue. Would be nice if we can solve this too.

devanshj commented 3 years ago

Yep it's already solved with the #662.

devanshj commented 3 years ago

it doesn't differentiate between a key not being present or a key being present

Can be fixed by using a stricter Partial ie type Partial<T> = { [K in keyof T]?: T[K] } and with --exactOptionalPropertyTypes set ;) Here's before and after.

Also isn't HandlerFn same as SetState<MyStore>? Unless I'm missing something? And that does compile.

TkDodo commented 3 years ago

Can be fixed by using a stricter Partial ie type Partial = { [K in keyof T]?: T[K] } and with --exactOptionalPropertyTypes set

isn't that the exact implementation of the build in type Partial?

type Partial<T> = {
    [P in keyof T]?: T[P];
};

it's true that the new compiler flag solves it, but exactOptionalPropertyTypes is an opt in setting and only available in TS4.4+. However, I would be totally fine with moving there and pointing people in the right direction, which would basically mean going back to Partial for the type definition. It seems that the workarounds we introduced with

has caused a bunch of other issues in edge-cases

devanshj commented 3 years ago

isn't that the exact implementation of the build in type Partial?

Nope, the built in has an extra | undefined as in it's T[K] | undefined instead of T[K].

going back to Partial for the type definition

Right, although it's still kinda undecided if we'll remove the existing workaround or not.

has caused a bunch of other issues in edge-cases

Are there other issues you're aware of (apart from this one)?

TkDodo commented 3 years ago

Nope, the built in has an extra | undefined as in it's T[K] | undefined instead of T[K].

what I posted above is what I've copied from lib.es5.d.ts 🤔 :

Screenshot 2021-11-16 at 16 05 35

Are there other issues you're aware of (apart from this one)?

closed ones, which we worked around with 4 overloads:

devanshj commented 3 years ago

Look like you're right, I just looked at the tooltip and assumed that's the definition haha.

image.

closed ones, which we worked around with 4 overloads:

What do you mean by 4 overloads though? Could you produce a snippet where the current types would fail?

TkDodo commented 3 years ago

initially, the "better Partial" type proposed by me was:

export type PartialState<T extends State, K extends keyof T = keyof T> =
  | (Pick<T, K> | T)
  | ((state: T) => Pick<T, K> | T)

which was then extended to:

export type PartialState<
  T extends State,
  K1 extends keyof T = keyof T,
  K2 extends keyof T = K1,
  K3 extends keyof T = K2,
  K4 extends keyof T = K3
> =
  | (Pick<T, K1> | Pick<T, K2> | Pick<T, K3> | Pick<T, K4> | T)
  | ((state: T) => Pick<T, K1> | Pick<T, K2> | Pick<T, K3> | Pick<T, K4> | T)

because of the issue linked above because the type K could not be properly inferred.

All in all, with the new compiler option available, we could just revert all of this, use the built-in Partial type again (like it was before #317) and educate users that if they want real type-safety, they need to use TS4.4 with exactOptionalPropertyTypes.

devanshj commented 3 years ago

Yeah I guess that would be a better way to go, the only problem is exactOptionalPropertyTypes isn't turn on when using tsc --init so very few users would have it on. But on the other hand these workarounds have it's own problems. Though I think let's go with Partial<T>.

dai-shi commented 3 years ago

I think I like exactOptionalPropertyTypes. I will try enabling it in zustand/jotai/valtio.

dai-shi commented 2 years ago

PartialState is already deprecated in v3.7.0. So, let's close this.