Exceptions are not thrown when frozen objects are changed by an Angular template about ngrx-store-freeze HOT 18 OPEN

Yes, I'm suggesting you not use ngModel because its not geared towards immutability and its easy to mutate your state accidentally. If you have to use ngModel instead of something like reactive forms, you'll have to use the method mentioned above of assigning the value of the payload to a new object before dispatching. I would also suggest looking into using Reactive Forms, as they provide you the same access to form inputs, a model to which can mutate without mutating your store, and a stream of the form values.

from ngrx-store-freeze.

Ok, I see. If you could open a new issue with a small reproduction that would help. I don't want this issue to get derailed if possible.

from ngrx-store-freeze.

I ran into this same issue today. When using 2-way binding in a form that is given an object returned from the frozen store, the bindings cease to work. Meaning if I edit the form inputs and submit the form, none of the form value updates went bound back into the model. I did expect the update to fail, but I did not expect it to fail silently, but rather throw an error.

If I add a new property to an item frozen in the store, I get a proper error.

EXCEPTION: Error in ./DatatableComponent class DatatableComponent - inline template:29:8 caused by: Can't add property $$index, object is not extensible

I would expect that editing a property value of a frozen object would also throw an error. With this subtle ignoring of the value change attempt and the app pretends to work, freeze is not helping me understand that my app is trying to improperly update state. Can this be fixed? If object.freeze makes this impossible, then store freeze should use another means to freeze objects.

Thanks.

from ngrx-store-freeze.

@AnimaMundi Cau you provide code from reducer with action.type: 'login'?

from ngrx-store-freeze.

@AntiCZ There's no reducer, the action is caught by a side effect

from ngrx-store-freeze.

+1 Just ran into this subtle gotcha today as well. It's easy to work around but hard to notice as others have pointed out.

from ngrx-store-freeze.

To check changes fired in template too, i use this meta reducer:

function checkStore(reducer): ActionReducer<any> {
	let savedState = JSON.stringify({});
	return function (state = {}, action) {
		if (savedState !== JSON.stringify(state)) {
			throw new Error("State changed! Previous: " + savedState + " Actual: " + JSON.stringify(state));
		}
		const nextState = reducer(state, action);
		savedState = JSON.stringify(nextState);
		return nextState;
	};
}

from ngrx-store-freeze.

@diegomaninetti Thanks for sharing, but this meta reducer doesn't solve the problem. The issue is no changes are fired in the template once data references from the template has been passed through the store. Any further attempts to change the value of those references fall on frozen data which does not throw any errors.

from ngrx-store-freeze.

@AnimaMundi I konw: I'm using my meta reducer instead of ngrx-store-freeze one.
In this way store data is not frozen but any changes outside the reducers throws the exception.

from ngrx-store-freeze.

Just added a plunkr demo: https://plnkr.co/edit/K7sWjJ?p=preview

from ngrx-store-freeze.

@diegomaninetti Sorry I got the impression this was to be used in conjunction, now that makes perfect sense! It's a nice solution, however using immutable/frozen data comes with other benefits too regarding memoization and change detection and we would lose out on them. I don't think change detection advantages will apply though since I imagine anybody using Ngrx is also using the OnPush change detection strategy in Angular. But we also lose the ability to enforce our "pure" contract within the reducers, as with your reducer there is nothing to stop a developer from directly manipulating the current state within the reducer and returning that modified state rather than returning a modified copy of the current state. So I guess there are pros and cons to both solutions.

from ngrx-store-freeze.

Just realized this was happening to me as well today. Its fine if pass it a new object from the component, but that seems a bit band-aid-ish.
This seems to keep freeze from sealing the objects and breaking zone change detection.

// Component
public submit(): boolean {
        this.store.dispatch(new UserSpace.LoginAction(Object.assign({}, this.localState)));
        return false;
    }

from ngrx-store-freeze.

Why would you use ngModel in combination with @ngrx/store? Seems like an anti-pattern to use 2-way data binding when the Redux pattern is about one-way data flows coming from the Store.

from ngrx-store-freeze.

Are you suggesting to not use ngModel at all? If we didn't use it at all then how would we retrieve input values from form elements? We would need to directly access DOM elements and this would also be an anti-pattern.

Flux might be about about one-way data flows coming from the store, but it's also about one-way actions going to the store and these actions can be triggered by user events such as an input value changing or a form being submitted. In this case we need data flow from the view layer to the controller. Whether we use ngModel or direct DOM access, there is still 2 way data flow between the view layer and the controller.

from ngrx-store-freeze.

@brandonroberts

I have to report that the problem is not only with NgModel.

Just had a mutation mistake in a function that was passed to select. StoreFreeze didn't catched it but prevented the mutation!

If you build a production build without freeze, the error will occur again and cause a lot of confusion! Therefore store freeze currently gives false security.

from ngrx-store-freeze.

@CanKattwinkel this is not a silver bullet to prevent you from making mistakes. Using it in production has performance costs you don't need to pay in your production build. You use it in development to catch these issues before then.

from ngrx-store-freeze.

I think I haven't expressed myself quite correctly. Let me try again:

I only use freeze during development. There was an mistake within a select method. An object was mutated. Freeze prevented the mutation while dev. But it didn't throw an exception.

In the production environment without freeze, the mutation was then no longer suppressed and application errors flew as a result.

from ngrx-store-freeze.

I get around this issue by adding an @Input() setter to all my presentational form components that makes a clone of the object passed in (using lodash's clone or cloneDeep). Thus, if a frozen object from the store is passed in, it will be copied to a mutable "draft" object where it is editable by ngModel.

// example of an order entry form
@Input('order') set orderInput(value: Order) {
    if(value) {
      this.order = cloneDeep(value);
    } else {
      this.order = new Order();
    }
  }
  order = new Order();

Reactive forms are nice, but sometimes they are overkill and a simple template-driven form would be much easier to understand/change.

Redux/ngrx is about shared immutable state, in my opinion. Local state need not be immutable and doing so often adds more complexity than is needed.

from ngrx-store-freeze.