Exceptions are not thrown when frozen objects are changed by an Angular template

[nathanosdev]

I realise this is more than likely going to be an issue on the side of Object.freeze() rather than this module but I was hoping by posting here there might be something that the module could do to compensate.

Unfortunately I was caught in a bad way by the freezing of action payloads (I've opened a separate issue to make this clearer for others) and spent a long couple of hours debugging this.

I have an object with 2 properties which are bound to 2 separate ngModels.
I pass this object directly as the payload for an action and it then becomes frozen by this module,
so any further changes in the view no longer affect the frozen object in the controller and there are no exceptions thrown so no indication that the bug is caused by the object being frozen.

Using Object.assign to return a new object to use as the payload resolves the issue easily,
but knowing where the issue arose from in the first place is the tricky part.

Template:

<input [(ngModel)]="credentials.email">
<input [(ngModel)]="credentials.password">

Controller:

import { Component } from '@angular/core';
import { Store } from '@ngrx/store';

@Component({
  selector: 'login',
  template: require('./login.component.html')
})
export class LoginComponent {
  public credentials: any = {};

  constructor(
    private store: Store<any>
  ) { }

  /*
   * once this function is called,
   * updates to the view are no longer reflected in the controller
   */
  public submit = () => {
    this.store.dispatch({
      type: 'login',
      payload: this.credentials
    });
  }
}

  /*
   * using Object.assign() like this fixes the issue
   */
  public submit = () => {
    this.store.dispatch({
      type: 'login',
      payload: Object.assign({}, this.credentials)
    });
  }

[michael-lang]

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.

[AntiCZ]

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

[nathanosdev]

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

[WonderPanda]

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

[diegomaninetti]

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;
	};
}

[nathanosdev]

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

[diegomaninetti]

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

[diegomaninetti]

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

[nathanosdev]

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

[Kaffiend]

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;
    }

[brandonroberts]

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.

[nathanosdev]

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.

[brandonroberts]

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.

[CanKattwinkel]

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

[brandonroberts]

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

[CanKattwinkel]

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.

[brandonroberts]

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.

[jongunter]

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.