fix: types for optional stores

[filoozom]

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update [x] Bug fix [ ] New feature [ ] Other, please explain:

What changes did you make? (Give an overview)

Changed the types of getStoreType, useStoreType, setStoreType and withStoreType to accept stores where some keys are optional.

Which issue (if any) does this pull request address?

I didn't open an issue beforehand, but I'll try to explain the issue. Imagine we have the following store:

type User = {
  username: string;
  avatar: string | undefined;
}

type Store = {
  user: User | undefined
}

const { useStore, getStore, withStore, setStore } = createStore<Store>()

The following function calls are marked as invalid by TypeScript, but do work through the Proxy:

setStore.user.username() // Property 'username' does not exist on type 'setter<User | undefined>'.
getStore.user.username() // Property 'username' does not exist on type 'HookDry<User | undefined>'.
useStore.user.username() // Property 'username' does not exist on type 'Hook<User | undefined>'.
withStore.user.username() // Property 'username' does not exist on type 'HocFunc<Store, ComponentClass<{}, any>>'.

Is there anything you'd like reviewers to focus on?

I'm not sure this is the best way to do things, and I'm also not too sure how you'd prefer your types to be for getStore, withStore and useStore for example. This PR changes those as such:

const [username, setUsername] = getStore.user.username()

typeof username = string;
typeof setUsername = setter<string>
typeof getStore.user = useStoreType<User> & HookDry<User | undefined>

I think at least username should be string? instead of string or getStore.user should be Optional<useStoreType<User> & HookDry<User | undefined>>. The latter makes less sense to me, as technically the code runs and doesn't throw a Cannot read properties of undefined (reading 'user') error, but the fact that username might be undefined as long a one of its parent keys is undefined should be clear.

[aralroca]

Thank you for your contribution @filoozom.

Before reviewing the PR, would you confirm if the same issue is happening in 0.11.0-canary.2 prerelease? Thank you!

We were working to migrate the project to TypeScript in this PR https://github.com/teafuljs/teaful/pull/68 (needs to be merged yet, but we can merge it at any time so we can work better on the new types), and we take the opportunity to fix some issues with types. After this change, the tests check the types, probably we can add this failing case.

[filoozom]

Hi @aralroca, thank you for your fast answer! I'm afraid it does still happen with the latest version.

[aralroca]

@filoozom I just merged the PR migrating to TypeScript and will do the 0.11 release once what you are talking about works. In the end, whether or not it works for you with the PR I've merged now, it would be nice to pass the new tests with this bug in mind. Sorry if that means you have to fix some conflicts and add the test. Thank you very much for your contribution!

[aralroca]

@all-contributors please add @filoozom for code

[allcontributors[bot]]

@aralroca

I've put up a pull request to add @filoozom! :tada:

[aralroca]

@filoozom I released 0.11 with your change and more TypeScript changes! 👍