Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
luthor77 opened this issue Jun 14, 2019 · 11 comments

Comments

@luthor77
Copy link

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

@splincode
Copy link
Member

Solution

image

@luthor77
Copy link
Author

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
Copy link
Member

Show me stackblitz example for reproduce with immer-adapter 3.0.4

@luthor77
Copy link
Author

luthor77 commented Jun 16, 2019

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
Copy link
Author

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

@luthor77
Copy link
Author

luthor77 commented Jul 4, 2019

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

@splincode
Copy link
Member

@luthor77 what do you mean?

@luthor77
Copy link
Author

luthor77 commented Jul 8, 2019

@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.

@splincode splincode reopened this Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants