False positive when using decorators: Since strict-mode is enabled, changing (observed) observable values without using an action is not allowed.

[PerfectionVR]

Intended outcome:

No warnings about modifications outside of action.

Actual outcome:

[MobX] Since strict-mode is enabled, changing (observed) observable values without using an action is not allowed. Tried to modify: [email protected]()

How to reproduce the issue:

I made a codesandbox: https://codesandbox.io/s/blue-sound-h4y8s?file=/src/index.tsx
Press the buttons and see the console, without runInAction inside of the action, the observable map will generate a warning.

Versions

Using codesandbox.

Scenario
I had issues upgrading from MobX 5 to 6. Our project is quite complex and large, relying heavily on subclassing, decorators and other stuff. I'll share some code to see what it looks like. It's unclear to me if I'm doing something wrong.

import { action, computed, configure, makeObservable, observable, runInAction } from 'mobx';

// For this test it is crucial to enforce these mobx settings for testing.
configure({
  enforceActions: 'always',
});

interface TestItem {
  id: string;
  value: string;
}

class ObservableTest {
  /**
   * Observable value
   */
  @observable
  private _value = 'default';
  @action
  /**
   * Action to set value
   */
  public setValue(value: string) {
    this._value = value;
  }
  /**
   * Computed value getter
   */
  @action
  public get value(): string {
    return this._value;
  }
  /**
   * ObservableMap of items
   */
  private _items = observable.map<string, TestItem>();
  /**
   * Action to set to map
   * @param item TestItem to set
   */
  @action
  public setItem(item: TestItem) {
    runInAction(() => {
      this._items.set(item.id, item);
    });
  }
  /**
   * Action to remove from map
   * @param item TestItem to remove
   */
  @action
  public removeItem(item: TestItem) {
    runInAction(() => {
      this._items.delete(item.id);
    });
  }
  /**
   * Returns items in an array.
   */
  @computed
  public get items() {
    return Array.from(this._items.values());
  }
  /**
   * Default Constructor
   * Boostraps observables.
   */
  public constructor() {
    // Initialize observables.
    makeObservable(this, {});
  }
}

describe('MobX', () => {
  it('should value default', () => {
    const test = new ObservableTest();
    expect(test.value).toStrictEqual('default');
  });

  it('should set value', () => {
    const test = new ObservableTest();
    test.setValue('a'); 
    expect(test.value).toStrictEqual('a');
  });

  it('should item default', () => {
    const test = new ObservableTest();
    expect(test.items).toStrictEqual([]);
  });

  it('should set item', () => {
    const test = new ObservableTest();
    const item = { id: '1', value: 'a' };
    test.setItem(item); // triggers unexpected warning
    expect(test.items).toStrictEqual([item]);
  });

  it('should remove item', () => {
    const test = new ObservableTest();
    const item = { id: '1', value: 'a' };
    test.setItem(item); // triggers unexpected warning
    test.removeItem(item); // triggers unexpected warning
    expect(test.items).toStrictEqual([]);
  });
});

[mweststrate]

You've put strict mode to 'always', which will require constructors to be called from actions. That is why 'observed' is the better setting and default instead. See the docs on config for more details.

…

On Wed, Oct 20, 2021 at 10:25 AM PerfectionVR ***@***.***> wrote: *Intended outcome:* No warnings about modifications outside of action. *Actual outcome:* [MobX] Since strict-mode is enabled, changing (observed) observable values without using an action is not allowed. Tried to modify: ***@***.***() *How to reproduce the issue:* I made a codesandbox: https://codesandbox.io/s/blue-sound-h4y8s?file=/src/index.tsx Press the buttons and see the console, without runInAction inside of the action, the observable map will generate a warning. *Versions* Using codesandbox. *Scenario* I had issues upgrading from MobX 5 to 6. Our project is quite complex and large, relying heavily on subclassing, decorators and other stuff. I'll share some code to see what it looks like. It's unclear to me if I'm doing something wrong. import { action, computed, configure, makeObservable, observable, runInAction } from 'mobx'; // For this test it is crucial to enforce these mobx settings for testing.configure({ enforceActions: 'always',}); interface TestItem { id: string; value: string;} class ObservableTest { /** * Observable value */ @observable private _value = 'default'; @action /** * Action to set value */ public setValue(value: string) { this._value = value; } /** * Computed value getter */ @action public get value(): string { return this._value; } /** * ObservableMap of items */ private _items = observable.map<string, TestItem>(); /** * Action to set to map * @param item TestItem to set */ @action public setItem(item: TestItem) { runInAction(() => { this._items.set(item.id, item); }); } /** * Action to remove from map * @param item TestItem to remove */ @action public removeItem(item: TestItem) { runInAction(() => { this._items.delete(item.id); }); } /** * Returns items in an array. */ @computed public get items() { return Array.from(this._items.values()); } /** * Default Constructor * Boostraps observables. */ public constructor() { // Initialize observables. makeObservable(this, {}); }} describe('MobX', () => { it('should value default', () => { const test = new ObservableTest(); expect(test.value).toStrictEqual('default'); }); it('should set value', () => { const test = new ObservableTest(); test.setValue('a'); expect(test.value).toStrictEqual('a'); }); it('should item default', () => { const test = new ObservableTest(); expect(test.items).toStrictEqual([]); }); it('should set item', () => { const test = new ObservableTest(); const item = { id: '1', value: 'a' }; test.setItem(item); // triggers unexpected warning expect(test.items).toStrictEqual([item]); }); it('should remove item', () => { const test = new ObservableTest(); const item = { id: '1', value: 'a' }; test.setItem(item); // triggers unexpected warning test.removeItem(item); // triggers unexpected warning expect(test.items).toStrictEqual([]); });}); — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3152>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBFPBXEUCPZ3PHZUCB3UH2DHRANCNFSM5GLEU5KQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[PerfectionVR]

You've put strict mode to 'always', which will require constructors to be called from actions. That is why 'observed' is the better setting and default instead. See the docs on config for more details.
…

I assume you mean this piece of text: https://mobx.js.org/configuration.html#enforceactions

Assume

  /**
   * ObservableMap of items
   */
  private _items = observable.map<string, TestItem>();

Meaning that @action will do nothing in this scenario.

  /**
   * Action to remove from map
   * @param item TestItem to remove
   */
  @action
  public removeItem(item: TestItem) {
      this._items.delete(item.id);
  }

It should be done using runInAction.

  /**
   * Action to remove from map
   * @param item TestItem to remove
   */
  public removeItem(item: TestItem) {
    runInAction(() => {
      this._items.delete(item.id);
    });
  }

And using enforceActions: 'observed' makes the following correct?

  /**
   * Action to remove from map
   * @param item TestItem to remove
   */
  public removeItem(item: TestItem) {
      this._items.delete(item.id);
  }

[urugator]

You must not pass the second arg to makeObservable(this) when using decorators.

[urugator]

Just to clarify, decorators in your example were simply ignored, because you passed (empty) annotations object. The enforceAction works the same regardless of @action/runInAction/action/... Everything should be more or less the same as in V5, you just have to call makeObservable(this) in constructor.
In the next mobx version you should get a friendly error (#3154)

[PerfectionVR]

I appreciate the swift help.