Typescript issues with slices pattern

[connormanning]

It looks to be common that Typescript issues are submitted without following the TS docs, but hopefully this is not is the case here since I am trying to follow those docs. There are 2 issues that I'm seeing when following these TS docs for the slice pattern:

• Additional keys in a state-setting object literal are allowed
• With the immer middleware mutator added to the slice, the state type is not inferred properly

Here is a reproduction repo with zustand@4.5.2. index.ts is almost exactly the same as the slices pattern TS doc, with a couple of tweaks:

• an invalid key is added to the state object
• with the immer mutator, an invalid state is set

Both of these "errors" compile successfully with no warning (and strict: true in tsconfig.json). Are these behaviors expected? Is something wrong with the code? I tried to keep the changes minimal from the official docs.

Searching this forum for similar posts I came across this code sample. This exact code, with some bad state setters added, does not error for me with current zustand.

Is it possible there have been some changes to the slices pattern for TS that have not been reflected in the docs? Maybe I'm missing something obvious but I couldn't figure it out.

Thanks for any insight!

[dai-shi]

https://tsplay.dev/wX0aVw
The first one is probably a limitation of TypeScript.
The second one seems working in the playground.

[connormanning]

Typescript in general can detect this error, and in this context it's extremely useful since if you were to change the name of a key in the store, a bug would be created since the old key would be silently accepted. I'd think there's an extends applied incorrectly in a type somewhere, but I have not debugged yet.

type WithValue = { value: number }
function doubleit({ value }: WithValue) { return value * 2 }

// This one compiles since it's in variable format, fromVariable matches WithValue.
const fromVariable = { value: 2, BADKEY: 42 }
doubleit(fromVariable) 

// ERROR, in the object literal form, we cannot pass additional keys.  This is what I'm referring to.
doubleit({ value: 2, BADKEY: 42 })

// However *this* version of the function would break this functionality since we've explicitly extended the type
// function doubleit<T extends WithValue>({ value }: T) { return value * 2 }

It's not clear to me what version of zustand the playground is running, but I did post an example repo that reproduces the issue. This repo was bootstrapped simply with npm install tsc && tsc --init && npm install zustand, so I'm assuming this is a regression from whatever version the playground runs to the current zustand version. I have no idea how to view the version or update zustand in the playground, but at the very least I can see that the StateCreator type is not the same as the current version of zustand, so it does not appear to be up to date:

// Zustand 4.5.2
type StateCreator<T, Mis extends [StoreMutatorIdentifier, unknown][] = [], Mos extends [StoreMutatorIdentifier, unknown][] = [], U = T> = ((setState: Get<Mutate<StoreApi<T>, Mis>, 'setState', never>, getState: Get<Mutate<StoreApi<T>, Mis>, 'getState', never>, store: Mutate<StoreApi<T>, Mis>) => U) & {
    $$storeMutators?: Mos;
};
// Zustand ?.?.? - Playground
type StateCreator<T, Mis extends [keyof StoreMutators<unknown, unknown>, unknown][] = [], Mos extends [keyof StoreMutators<unknown, unknown>, unknown][] = [], U = T> = ((setState: Get<Mutate<StoreApi<T>, Mis>, 'setState', never>, getState: Get<Mutate<StoreApi<T>, Mis>, 'getState', never>, store: Mutate<StoreApi<T>, Mis>) => U) & {
    $$storeMutators?: Mos;
}

It's certainly possible there's some configuration issue at play (which is why I posted a reproduction repo), but from what I can tell both of these items are real issues.

[connormanning]

I've made some progress here. The second issue is solved by installing immer explicitly in the project. From the readme, I would have expected immer to be a dependency of zustand, but it is listed as a devDependency. So npm install immer fixes the second issue. Maybe a note in the readme would be useful, and possibly this applies to the redux middleware as well.

As for the first issue, it appears if you are using the immer middleware, that the increment(state => ({ value: state.value + 1 })) pattern is not supported by the middleware type definition - only the void result mutating the Draft directly is allowed. However in reality, the other type still works, it is just not type-checked. The type for set with the immer middleware is:

setState(nextStateOrUpdater: T | Partial<T> | ((state: Draft<T>) => void)

So the immer middleware overwrites the type of (state: T) => T | Partial<T> with (state: Draft<T>) => void. Interestingly, since the middleware only seems to apply if you modify the draft, you can get yourself into trouble using the non-immer-based updater (state: T) => Partial<T> which was overwritten: here is a playground link demonstrating some bugs that compile cleanly.

I guess the question is whether you are still allowed to use the vanilla zustand set(state => ({ value: state.value + 1 })) format if you are using the immer middleware. Forgetting about TS types, it looks like this is intended to be allowed. If you modify the Draft and return nothing, then the immer middleware runs as expected. But if you return partial state object, that also works (but is untyped). If you do both, that's a runtime error which makes sense: caught Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.

If that's the expected behavior, then I think the immer middleware's setState type should probably be updated. A quick hack shows that this line might be modified:

// Current:
T | Partial<T> | ((state: Draft<T>) => void)

// Suggested:
T | Partial<T> | ((state: Draft<T>) => T | Partial<T> | void)

This catches all of the errors in my Playground link, by making sure that if you do return something that it is the right type, but I'm not 100% sure it's a fully robust solution or the optimal fix.

[dai-shi]

So npm install immer fixes the second issue.

yeah, immer is a peer dependency because not everyone uses it. I thought it's obvious, but let's document it somewhere.

this applies to the redux middleware as well.

it doesn't. there's no redux dependencies. it's just redux-like middleware.

But if you return partial state object, that also works (but is untyped).

Good point. I think there's a historical reason. Probably, we should drop supporting returning partial state object. (This is breaking, so let's do it in v5.)

[connormanning]

Probably, we should drop supporting returning partial state object

Interesting, so using immer middleware is intended to replace the Partial object model entirely? From the error message Either return a new value *or* modify the draft I thought the intention was that you could do one or the other, and that just the type was missing for the Partial one.

[dai-shi]

I checked the source code. I misunderstood something.
It's not that zustand that does it, but it's what immer does.

https://github.com/immerjs/immer/blob/10971031f61286d634f24cba6374016cc27c37b8/src/types/types-external.ts#L86-L90

So, I guess your suggested fix is correct. Please open a PR.

[connormanning]

Opened #2495.

[dai-shi]

Probably, we should drop supporting returning partial state object

Yeah, I was probably referring to it:
#2481 (comment)