ngxs-labs / immer-adapter

:hamster: Declarative state mutations
MIT License
46 stars 10 forks source link

When state is null/undefined ImmutableSelector/ImmutableContext stops working. #234

Open luthor77 opened 5 years ago

luthor77 commented 5 years ago

When the immer-adapter was using 'produce' as the mutate operator, in some cases the draft was undefined, and I could hadle it with an if. But now it just gives errors, even on Selectors, that I didn't had to handle before.

First I don't know if I should report separately but, when resetting the state using defaults or an object with the type of the State, the state's children are deleted, this is the most common cause of the undefined state. (Sometimes isn't needed to be a children of a reseted state to the state become undefined, but I couldn't reproduce what happens in my application)

The worst for me is the impossibility to handle this when I use immer-adapter.

Here is the stackblitz exemple: https://stackblitz.com/edit/angular-6gaame

Testing this repository I found a new error too, when using a ImmutableContext on a Receiver, other receivers on that state can't mutate the state without ImmutableContext.

splincode commented 5 years ago

If you do not use immer, then you are not allowed to mutate the state. And that's correct behavior in NGXS.

Correct immutable state without Immer

 @Action(Add)
  public add({ getState, setState }: StateContext<AnimalsStateModel>, { payload }: Add): void {
    setState((state: AnimalsStateModel) => ({
      ...state,
      zebra: {
        ...state.zebra,
        food: [ ...state.zebra.food, payload ]
      }
    }));
  }

Correct immutable state with Immer mutation

  @Action(Add)
  @ImmutableContext()
  public add({ setState }: StateContext<AnimalsStateModel>, { payload }: Add): void {
    setState((state: AnimalsStateModel) => ({
      state.zebra.food.push(payload);
      return state;
    }));
  }
splincode commented 5 years ago

In your example, you are breaking the rules for using the NGXS.

 @Receiver({ type: '[Child] Set Ok Status Without Immer' })
  public static setOkStatusWithoutImmer({ setState }: StateContext<ChildModel>) {
    setState((draft: ChildModel) => {
      console.log(draft)
      if (draft) {
        draft.status = draft.status === 'OK' ? defaults.status : 'OK'

        return draft
      }
      return {...defaults}
    })
  }

You are trying to mutate the state directly and this is not correct. Because then the NGXS check is triggered.

https://github.com/ngxs/store/blob/1a85af3ec36b669bb7491332cf62fc6db202e955/packages/store/src/internal/state-operations.ts#L46

splincode commented 5 years ago

https://medium.com/ngxs/immutable-state-in-ngxs-part-i-ba318bfc5bb3

splincode commented 5 years ago

Solution

image

luthor77 commented 5 years ago

Ok, this helps me with the last part that:

Testing this repository I found a new error too, when using a ImmutableContext on a Receiver, other receivers on that state can't mutate the state without ImmutableContext.

But if I'm not wrong, doesn't answer me the major problem. That is, immer-adapter now, just break when the state is undefined, that is often caused by:

when resetting the state using defaults or an object with the type of the State, the state's children are deleted

I don't know if this is the appropriated way to reset a state with childrens, or if it is and the childrens shouldn't be deleted. But this happens and:

When the immer-adapter was using 'produce' as the mutate operator, in some cases the draft was undefined, and I could hadle it with an 'if'. But now it just gives errors, even on Selectors, that I didn't had to handle before.

The error is the same for ImmutableContext and ImmutableSelector.

And last:

Sometimes isn't needed to be a children of a reseted state to the state become undefined, but I couldn't reproduce what happens in my application.

So I really need that the immer-adapter don't break when the state is undefined.

splincode commented 5 years ago

Show me stackblitz example for reproduce with immer-adapter 3.0.4

luthor77 commented 5 years ago

https://stackblitz.com/edit/angular-pzgz7n

I commented the parts that I think it's important, sorry if all is just a wrong implementation made by me.

luthor77 commented 5 years ago

I updated the immer-adapter version in this example to 3.0.5.

luthor77 commented 5 years ago

@splincode I'd like to have some feedback about this.

splincode commented 5 years ago

@luthor77 what do you mean?

luthor77 commented 5 years ago

@splincode, You closed the issue without reading all I wrote (You read only the last two lines, that wasn't even related to the title, it was just a wrong observation made by me), then I said all again and you asked me a new reproduce example with the updated version of immer-adapter, I made. And I'm waiting until now.