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

Make set & get types use generic state #35

Merged
merged 1 commit into from
Jun 24, 2019
Merged

Conversation

JeremyRH
Copy link
Contributor

Hopefully this fixes #32 and still allows type inference in JS files.
@jfbrazeau @drcmda @Magellol @dm385

@drcmda
Copy link
Member

drcmda commented Jun 23, 2019

I'm not knowledgable about TS enough to review this, you're the pro 🙂 but if it solves the issue, let's merge.


export default function create<TState extends State>(
createState: (set: SetState<State>, get: GetState<State>, api: any) => TState
createState: keyof TState extends never
Copy link

@Magellol Magellol Jun 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand, is it to detect if we're in JS vs TS land? I'm curious about that extends never bit.

I just tested it and it does fix the type inference issue in TS. We still have to pass the state generic explicitely to create to not get implicit anys but I think that's a good DX behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand, is it to detect if we're in JS vs TS land?

Essentially yeah. It checks to see if the state generic was passed in or not.

Copy link

@jfbrazeau jfbrazeau Jul 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
The fix works fine with a very basic sample like this one :
Screen Shot 2019-07-03 at 17 55 54

But I'm afraid that th fix is not enough because it is very easy to get a non working case. Imagine you have several stores, and imagine you want to do something generic on them. You will create a function that performs the "generic" things and then call that function for each store you have.

This will lead you to create a generic function that encapsulates the zustand create function invocation. But as far as I can see, it doesn't work.

Just take a look at this really trivial sample in which we try to do something generic during store intialization :
Screen Shot 2019-07-03 at 17 56 45

I hope you will agree with me that this sample code is trivial (10 lines of effective code), and it highlights the problem...

In other words, as soon as you want to add even a thin layer over zustand for your own purpose in Typescript world, you meet that problem.

So on my side, I still have the same issue as the one I raised in issue #32.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have put all this (trivial) code on codesandbox : https://codesandbox.io/s/j2nys
(basic.ts and initStore.ts files)

Copy link

@Magellol Magellol Jul 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I've tested without passing the generic TState from initStore and it works.

interface State {
  foo: string;
}

export function initStore(initialState: State) {
  return create<State>((set: SetState<State>, get: GetState<State>) => {
    return {
      foo: "bar"
    };
  });
}

But Typescript complains as soon as we pass a generic down the line:

export function initStore<S>(initialState: S) {
  return create<S>((set: SetState<S>, get: GetState<S>) => {
    return {};
  });
}

It looks like the conditional type isn't kicking in correctly when a generic is used.

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.

SetState & GetState Typescript declaration in create function arguments
4 participants