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

Fix SetState & GetState Typescript declaration in create function arg… #31

Closed
wants to merge 1 commit into from

Conversation

jfbrazeau
Copy link

@jfbrazeau jfbrazeau commented Jun 12, 2019

The create function typescript definition has a generic parameter that helps to define the final state that will be used : TState extends State. This state has to extend State (Record<string, any>) which is relevant for a state.

Unfortunately both set and get parameters types don't use this generic parameter and instead are bound to State as we can see :
createState: (set: SetState<State>, get: GetState<State>, api: any) => TState

It is not very anoying for the set parameter, but it is really a pity that when one uses the get method, the definition does not simply type the return to TState instead of the very neutral State.

Is it just a mistake ? Or is there a reason not to have used TState in set: SetState<State> and get: GetState<State> ? (I must admit that I would expect set: SetState<TState> and get: GetState<TState> instead).

@Magellol
Copy link

Magellol commented Jun 12, 2019

This is similar to a PR I had opened (#27) but I believe this change breaks autocompletion for non typescript users.

I think the workaround is to do

create<MyState>((set: SetState<MyState>, get: GetState<Mystate>) => {}})

@drcmda
Copy link
Member

drcmda commented Jun 21, 2019

@JeremyRH @Magellol @jfbrazeau could we discuss this again? We just ran into it as well. Is it important if non TS users get broken intellisense? TBH, as a non TS use im kinda used to nothing giving me autocomplete. But now that i work on a TS project, passing on set/get into a factory, it's a little annoying the way it is and this PR would help it.

@jfbrazeau
Copy link
Author

jfbrazeau commented Jun 21, 2019

This is similar to a PR I had opened (#27) but I believe this change breaks autocompletion for non typescript users.

I think the workaround is to do

create<MyState>((set: SetState<MyState>, get: GetState<Mystate>) => {}})

I'm afraid I don't understand your workaround.

I've tried it to see what it gives and here is the result :
Screen Shot 2019-06-21 at 16 48 51

As you can see I've described a very simple state (MyState) and applied your workaround. Typescipt complains that (as you can see in the 'PROBLEMS' view of VSCode (see screenshot) :
Argument of type '(setState: SetState<MyState>, getState: GetState<MyState>) => MyState' is not assignable to parameter of type '(set: SetState<Record<string, any>>, get: GetState<Record<string, any>>, api: any) => MyState'. Types of parameters 'getState' and 'get' are incompatible. Type 'Record<string, any>' is missing the following properties from type 'MyState': stateParm1, stateParm2, anAction

The (very unsatisfying) workaround I found consists in duplicating the zustand defintion file (index.d.ts) in my module and configure Typescript to use it to override zustand default definitions (in tsconfig.json) ; in other words, this line :

export default function create<TState extends State>(createState: (set: SetState<State>, get: GetState<State>, api: any) => TState): [UseStore<TState>, StoreApi<TState>];

becomes in my environment this one :

export default function create<TState extends State>(createState: (set: SetState<TState>, get: GetState<TState>, api: any) => TState): [UseStore<TState>, StoreApi<TState>];

When you are a Typescript user, the default zustand definition is very strange. The create function is parameterized with TState. That function returns itself a TState, but it provides setter and getter for.... State instead of TState !... So if you plan to use this setter/getter pair, you will have to cast the state as soon as you want to use it. Very strange when you consider that you already gave the state type through the TState parameter.

@jfbrazeau
Copy link
Author

Here is a workaround that seems to work :
Screen Shot 2019-06-21 at 17 06 25

You have to cast the state every time you want to use it.

Possible workaround, but which is not relevant as far as I can see. Unfortunately I prefer to duplicate the zustand definition file to get rid of that problem (even if this is not satisfying me at all).

(But I must admit that if I omit tlittle typescript issue, I really like zustand, so for now I accept my unsatisfying workaround... ;) )

@dm385
Copy link

dm385 commented Jun 21, 2019

Could some kind of function overloading solve the problem? Or the definition of a second, typed function to create a store?

export function createT<TState extends State>(
  createState: (set: SetState<TState>, get: GetState<TState>, api: any) => TState
): [UseStore<TState>, StoreApi<TState>] {
  return create<TState>(createState as any);
}

Such a "wrapper" function could even be implemented in the application code, so you would not have to cast every time.

@drcmda
Copy link
Member

drcmda commented Jun 24, 2019

closed in favour of #35

@drcmda drcmda closed this Jun 24, 2019
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

Successfully merging this pull request may close these issues.

None yet

4 participants