esamattis / immer-reducer

Type-safe and terse reducers with Typescript for React Hooks and Redux
http://npm.im/immer-reducer
MIT License
225 stars 15 forks source link

Returning new object from reducer does not introduce new state #23

Closed nkovacic closed 5 years ago

nkovacic commented 5 years ago

Returning a new object from a reducer function does not reset the state to thjs object. This should happen according to immer docs https://github.com/mweststrate/immer#returning-data-from-producers

You can check out the issue in this stackblitz example reducer

esamattis commented 5 years ago

It's by design. Very early version actually had that feature.

I removed it because typing wise it's not very strict.

For example if you have state like

interface State {
  ding: string
}

And you write a reducer method like

class Reducer extends ImmerReducer<State> {
  setDing() {
    return { ding: "str", wat: "not actually in your state interface" };
  }
}

It does not produce a typing error because it's still compatible with state interface even if wat does not exists in it and you'll get untyped stuff to your redux state. It's just how stuctural typing works in TypeScript.

I'm not completely against this feature if somebody would sent a PR with both runtime and type tests but I'm not going to implement it myself since I'm not going to use it personally.

nkovacic commented 5 years ago

Reducer function should return State | undefined type. Would that not solve the typing issues?

esamattis commented 5 years ago

No :(

Here's a super simplefied version of the issue with ImmerReducer:

https://stackblitz.com/edit/typescript-ky5bvg

No type error. ImmerReducer is basically the same but for classes.

nkovacic commented 5 years ago

Followed up on this type issue, found this stackoverlow that could solve it. A very crud version of it which does not honor non-required properties and their types is on this stackblitz

esamattis commented 5 years ago

I don't understand. It still does not error properly here

kuva

And you must manually add the state return type...

I'd rather allow setting the draftState object directly than returning new state.

Eg.

this.draftState = initialState;
esamattis commented 5 years ago

You can do it even now with Object.assing():

Object.assign(this.draftState, initialState)
nkovacic commented 5 years ago

We just have a usercase where we reset the draftstate back to initial. Is there any way this could be done in immer-reducer without doing it manually? Redeclaring draftState is unfortunately not supported in immer (this.draftState = initial).

nkovacic commented 5 years ago

Also postead an issue/proposal in immer.

esamattis commented 5 years ago

For immer-reducer it can (or must) itself implement it.

esamattis commented 5 years ago

Implemented in v0.7.5