New computed decorator does not apply action to setter

[olee]

It seems that with the new decorators, setters for computed getters do not become actions anymore, as it is also mentioned in the documentation:

It is possible to define a setter for computed values as well. Note that these setters cannot be used to alter the value of the computed property directly, but they can be used as an "inverse" of the derivation. Setters are automatically marked as actions

The code at the bottom works fine with old decorators, but with will report the following error:

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

Also, it seems there is no way to use like an action decorator on the getter to intentionally make it an action.

How to reproduce the issue:

class Store {
    @observable
    accessor value = 1;

    @computed
    public get doubleValue() {
        return this.value * 2;
    }

    public set doubleValue(value: number) {
        this.value = value / 2;
    }
}

const store = new Store();
autorun(() => console.log(store.doubleValue));
store.doubleValue = 32;

See also: https://github.com/mobxjs/mobx/blob/c22c904f30513887b805b82d6da56518ce8985b5/packages/mobx/src/types/computedannotation.ts#L50-L73

[Matchlighter]

(Implementation note/context - not really an answer) This is a little tricky with the 2023.x decorators spec. Getters and Setters may now have separate decorators applied to them - it's no longer the case that a decorator on one (eg getter) applies to the other (eg setter). The spec has grown enough that MobX may be able to workaround this now (using addInitializer), but I believe it'd have to redefine the property and would break some optimizations that occur in the JS engine. It would also increase integration issues with other (non-MobX) decorators. An alternative would be to allow @action to be applied to setters. Less implicit, but probably more semantic?

[mweststrate]

Setters are automatically marked as actions

This was a bad idea anyway, so probably better to keep it as is :). @action should be on the "outside", that is the event sources (on-click, websocket on-message etc), so that they span an entire "tick" of the application, not at the level of individual attributes where they are largely meaningless.

[olee]

Then I'd suggest we allow applying @action to setters as well, because right now you'd have to do this:

    public set language(value: string | undefined) {
        runInAction(() => {
            this.#language = value;
        });
    }

And even if you don't intend to allow @action on setters, the documentation on the website should be updated imho.

[mweststrate]

the documentation on the website should be updated imho.

agreed

because right now you'd have to do this:

Per above, the main guidance is that you make whatever calls this language setter an action, not the setter. One way to think about it: If MobX didn't allow nesting action, what is the most outer function where you could put the the action on? That is the right place. An action for an individual attribute is basically a pointless transaction. You want all assignments that need to happen to be wrapped in an action (which in cases can still be only one of course).

Since you cannot make in JavaScript a field assignment a first class callback of something, there should be a different place where putting action is more appropiate. For example: on-click={action(e ==> { store.language = "en_US" })}. This scales, because if you make that on-click handler more complicated, and add an another assignment to it, you now end up with a single transaction, rather than two, which would make MobX partially invalidate the dep tree of the signals also twice.

[olee]

This is however different to how setters behave when using observable({ ... }) or makeAutoObservable(this) which make setters into actions