Improve type definition of `create` to accept `CustomSetState` #428

hachibeeDI · 2021-06-12T05:33:03Z

Motivation

I was creating a middleware which returns StateCreator<State, CustomSetState> but it was denied because create is not accept CustomSetState as a second type argument.

Bonus

As you noticed, I modified a test "can set the store without merging" in basic.test.tsx.

Previously StateCreator couldn't infer the state correctly so that store was inferred as object.

// const setState: <never, never, never, never>(partial: PartialState<object, never, never, never, never>, replace?: boolean | undefined) => void
setState({ b: 2 }, true)

// const getState: () => object
getState()

But now it is as it should so we need let TypeScript compiler knows that keys a and b are possibly undefined.

Thanks!

codesandbox-ci · 2021-06-12T05:33:52Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4aa50f1:

Sandbox	Source
React	Configuration
React Typescript	Configuration

dai-shi · 2021-06-12T11:26:14Z

Hi, thanks for your work! This looks like a good improvement. Does this solve some existing issues?

I haven't looked into details yet, but I think we should keep the JS bundle size. Just added a commit to update the size snapshot. Can you change it to make changes only in TS?

hachibeeDI · 2021-06-13T04:59:26Z

Hello! Yes, my issue is going to be solved.
Also, I guess the issue #421 is related to this patch. That error says "Types of parameter 'set' and 'set' are incompatible.

Sure thing! Some of my extra nit pick makes size up. I'll fix it.

hachibeeDI · 2021-06-13T05:15:13Z

I confirmed now the JS bundle size is as it was. 👍

dai-shi

Bundle size is good now. Please check two comments.

dai-shi · 2021-06-13T11:43:56Z

src/vanilla.ts

-      typeof partial === 'function'
-        ? (partial as (state: TState) => TState)(state)
-        : partial
+    const nextState = typeof partial === 'function' ? partial(state) : partial


Hm, so we can remove the type assertion because of this PR, or was it already possible with the previous one?
Either way, let's remove L71-L72 comments.

Ah, sorry my fix was sloppy it wasn't revert correctly but... it work okay. Sure, gonna remove comments.

dai-shi · 2021-06-13T11:45:00Z

src/vanilla.ts

@@ -109,6 +119,7 @@ export default function create<TState extends State>(

  const destroy: Destroy = () => listeners.clear()
  const api = { setState, getState, subscribe, destroy }
-  state = createState(setState, getState, api)
+  // SetState<TState> includes CustomSetState so it should be ok


Can we do something like this?

diff --git a/src/vanilla.ts b/src/vanilla.ts index ed33bc2..38d1fd7 100644 --- a/src/vanilla.ts +++ b/src/vanilla.ts @@ -62,7 +62,7 @@ export type StateCreator<T extends State, CustomSetState = SetState<T>> = ( export default function create< TState extends State, - CustomSetState = SetState<TState> + CustomSetState extends SetState<TState> = SetState<TState> >(createState: StateCreator<TState, CustomSetState>): StoreApi<TState> { let state: TState const listeners: Set<StateListener<TState>> = new Set() @@ -119,7 +119,6 @@ export default function create< const destroy: Destroy = () => listeners.clear() const api = { setState, getState, subscribe, destroy } - // SetState<TState> includes CustomSetState so it should be ok - state = createState(setState as any as CustomSetState, getState, api) + state = createState(setState as CustomSetState, getState, api) return api }

Ahh,I should have figure it out before using any... Thank you for good catch!

Hm, like a test I added, I'd like to apply a CustomSetState that set a different type of state.
In a example, set cannot update readA/B because the second argument is SetState<Pick<ExampleState, 'a' | 'b'>>. However, Pick<ExampleState, 'a' | 'b'> couldn't satisfies TState...

CustomSetState extends SetState<any> = SetState<TState>

is going to fix setState as any as CustomSetState but test passes. I'll send a patch for your check.

Yeah, that's something I was concerned. Maybe this can't be helped.

SetState includes CustomSetState

I'm not sure if I understand this...

dai-shi · 2021-06-19T04:48:50Z

Not related, but linking #441.

dai-shi · 2021-06-20T22:53:10Z

src/vanilla.ts

-): StoreApi<TState> {
+export default function create<
+  TState extends State,
+  CustomSetState extends SetState<any> = SetState<TState>


I'm not sure what this any would cause. It could cause runtime error without compile error, technically?
@TkDodo can you review this PR please?

…and into enhance/custom-set-state

dai-shi · 2021-06-20T23:47:44Z

@hachibeeDI
How is this 9b65883 ?
Does it accomplish what you would like to do?

src/index.ts

src/vanilla.ts

dai-shi · 2021-06-21T08:18:22Z

Let me hold on this PR. ~~The type improvement seems good, so I'll create a separate PR.~~ (edit: maybe it doesn't mean much.)

dai-shi · 2021-07-07T23:51:36Z

My big apologies about adding some commits without any success. We should probably start over.
At this point, what we have tried seems to add any in the library code to avoid any in the app code, which doesn't make a lot of sense.
Or, I may misunderstand the original issue. Anyway, let's close this and hope someone to open a new discussion. I'm generally willing to improve types around CustomSetState.

hachibeeDI added 2 commits June 12, 2021 13:50

Use type guard function instead of type assert

2020a34

Now store creator able to receive CustomSetState

b5bf2cf

hachibeeDI and others added 3 commits June 12, 2021 16:00

Add a unit test to check CustomSetState is working

77cacd2

Merge branch 'master' into enhance/custom-set-state

15a7d39

update size snapshot

cfea6be

hachibeeDI added 2 commits June 13, 2021 14:06

revert changes which makes js bundle size up

14f1ea6

confirm my patch won't change snapshot size

403a780

dai-shi reviewed Jun 13, 2021

View reviewed changes

Decent type argument to remove type cast to any

546196e

dai-shi mentioned this pull request Jun 14, 2021

[v3.5.2] Type error when using devtools and persist in TypeScript #434

Closed

dai-shi mentioned this pull request Jun 19, 2021

make type NamedSet compatible with type SetState #441

Merged

dai-shi reviewed Jun 20, 2021

View reviewed changes

dai-shi added 3 commits June 21, 2021 07:54

Merge branch 'master' into enhance/custom-set-state

9e7dcb0

Merge branch 'enhance/custom-set-state' of github.com:hachibeeDI/zust…

28b44ff

…and into enhance/custom-set-state

wip: no any arg in src

9b65883

fix: tweak some more types

0a725a5

TkDodo reviewed Jun 21, 2021

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

src/vanilla.ts Outdated Show resolved Hide resolved

revert some changes

e239b08

Update src/vanilla.ts

4aa50f1

dai-shi closed this Jul 7, 2021

dai-shi mentioned this pull request Jul 19, 2021

useStore.subscribe hook does not correctly infer type from selector #331

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve type definition of `create` to accept `CustomSetState` #428

Improve type definition of `create` to accept `CustomSetState` #428

hachibeeDI commented Jun 12, 2021 •

edited

Loading

codesandbox-ci bot commented Jun 12, 2021 •

edited

Loading

dai-shi commented Jun 12, 2021 •

edited

Loading

hachibeeDI commented Jun 13, 2021

hachibeeDI commented Jun 13, 2021

dai-shi left a comment

dai-shi Jun 13, 2021

hachibeeDI Jun 13, 2021

dai-shi Jun 13, 2021

hachibeeDI Jun 13, 2021

hachibeeDI Jun 13, 2021

dai-shi Jun 13, 2021 •

edited

Loading

dai-shi commented Jun 19, 2021

dai-shi Jun 20, 2021

dai-shi commented Jun 20, 2021

dai-shi commented Jun 21, 2021 •

edited

Loading

dai-shi commented Jul 7, 2021

Improve type definition of create to accept CustomSetState #428

Improve type definition of create to accept CustomSetState #428

Conversation

hachibeeDI commented Jun 12, 2021 • edited Loading

Motivation

Bonus

codesandbox-ci bot commented Jun 12, 2021 • edited Loading

dai-shi commented Jun 12, 2021 • edited Loading

hachibeeDI commented Jun 13, 2021

hachibeeDI commented Jun 13, 2021

dai-shi left a comment

Choose a reason for hiding this comment

dai-shi Jun 13, 2021

Choose a reason for hiding this comment

hachibeeDI Jun 13, 2021

Choose a reason for hiding this comment

dai-shi Jun 13, 2021

Choose a reason for hiding this comment

hachibeeDI Jun 13, 2021

Choose a reason for hiding this comment

hachibeeDI Jun 13, 2021

Choose a reason for hiding this comment

dai-shi Jun 13, 2021 • edited Loading

Choose a reason for hiding this comment

dai-shi commented Jun 19, 2021

dai-shi Jun 20, 2021

Choose a reason for hiding this comment

dai-shi commented Jun 20, 2021

dai-shi commented Jun 21, 2021 • edited Loading

dai-shi commented Jul 7, 2021

Improve type definition of `create` to accept `CustomSetState` #428

Improve type definition of `create` to accept `CustomSetState` #428

hachibeeDI commented Jun 12, 2021 •

edited

Loading

codesandbox-ci bot commented Jun 12, 2021 •

edited

Loading

dai-shi commented Jun 12, 2021 •

edited

Loading

dai-shi Jun 13, 2021 •

edited

Loading

dai-shi commented Jun 21, 2021 •

edited

Loading