Search code examples
angularreduxngrx

Cannot assign to read only property on deep copied object


I am using angular 9 and ngrx 9

I have a selector that do like this :

    this.store$
      .pipe(select(SettingsStoreSelectors.selectNavigationSettings), take(1))
      .subscribe((settings: SettingsStoreModels.INavigationSettings) => {
        let copy = JSON.parse(JSON.stringify(settings));
        this.settings = copy.vrMapSettings;
      });

This is the only place settings is assigned in this component

In the component I then have a checkbox that edit a setting property

      <mat-checkbox
        class="setting-input"
        *ngIf="viewer"
        (click)="$event.stopPropagation()"
        (change)="onSettingChange(settings, true)"
        [(ngModel)]="settings.depthTestAgainstTerrain"
      >
        {{ "MAP.SETTINGS.DEPTH_TEST_AGAINST_TERRAIN" | translate }}
      </mat-checkbox>

This keeps triggering

core.js:4117 ERROR TypeError: Cannot assign to read only property 'depthTestAgainstTerrain' of object '[object Object]'

But I don't understand anymore what to do. I deep cloned my store property, I want to edit to resubmit an action but it's not possible. why...

EDIT : actually I found out the reason why but I do not understand how to use it correctly then.

I deep clone the object like above from my selector

When a setting change, I dispatch an action to change the setting object in the store : this action setting object reference the same object that is used by the component.

this.store$.dispatch(new SettingsStoreAction.SetNavigationSettings({ settings: { vrMapSettings: settings } }));

Which call this reducer :

const SET_NAVIGATION_SETTINGS = (state: State, action: featureAction.SetNavigationSettings) => {
  return {
    ...state,
    navigation: {
      ...state.navigation,
      ...action.payload.settings,
    },
  };
};

But then, if I edit again, since the store reference directly the setting object of the component, it trigger the error....

If I deep clone here too

this.store$.dispatch(new SettingsStoreAction.SetNavigationSettings({ settings: { vrMapSettings: JSON.parse(JSON.stringify(settings))} }));

The it works...

But if I have to create small action for EVERY property of settings, or deep clone for EVERY dispatch/selection of the store, there is going to be a HUGE flaw in the performance...

am I using it correctly ?

if need more information please comment I will provide.


Solution

  • You're right, a state should be mutated only via actions. In you case, if the settings object has a lot of properties, it might be inconvenient to create a different action for each of the properties. A copying of the settings looks like a solution but deep object copying complexity depends on the object size.

    I can suggest you use immer to create copies of the settings. Instead of copying the whole object it reuses all its unchanged parts that works much faster.

    ...
    <mat-checkbox
      class="setting-input"
      *ngIf="viewer"
      (click)="$event.stopPropagation()"
      (change)="onDepthTestAgainstTerrainChange($event)"
      [checked]="settings.depthTestAgainstTerrain">
      {{ "MAP.SETTINGS.DEPTH_TEST_AGAINST_TERRAIN" | translate }}
    </mat-checkbox>
    ...
    
    @Component(...)
    export class MyComponent {
      ...
      onDepthTestAgainstTerrainChange(change: MatCheckboxChange) {
        const patchedSettings = produce(settings, draft => {
          // draft remembers all the changes you've made and produces
          // new immutable state based on these mutations, but takes all
          // unchanged parts of the original object
          draft.depthTestAgainstTerrain = change.checked;
        });
    
        this.store$.dispatch(new SettingsStoreAction.SetNavigationSettings({
          settings: { vrMapSettings: patchedSettings}
        }));
      }
    }
    

    Or you can create an action which accepts a patch function, like:

      onDepthTestAgainstTerrainChange(change: MatCheckboxChange) {
        this.store$.dispatch(new SettingsStoreAction.PatchNavigationSettings(sttingsDraft => {
          sttingsDraft.depthTestAgainstTerrain = change.checked;
        }));
      }