Don't call Action while ComputedValue is computing

src/core/action.ts

@@ -60,6 +60,11 @@ export function executeAction(
     scope?: any,
     args?: IArguments
 ) {
+    if (__DEV__ && globalState.computingValue) {
+        console.warn(
+            `[MobX] Don't call Action ${actionName} while ComputedValue ${globalState.computingValue.name_} is computing`
+        )
+    }
     const runInfo = _startAction(actionName, canRunAsDeriviation, scope, args)
     try {
         return fn.apply(scope, args)

src/core/computedvalue.ts

@@ -217,6 +217,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
         this.isComputing_ = true
         // don't allow state changes during computation
         const prev = allowStateChangesStart(false)
+        const prevComputingValue = computingValueStart(this)
         let res: T | CaughtException
         if (track) {
             res = trackDerivedFunction(this, this.derivation_, this.scope_)
@@ -231,6 +232,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
                 }
             }
         }
+        computingValueEnd(prevComputingValue)
         allowStateChangesEnd(prev)
         this.isComputing_ = false
         return res
@@ -297,3 +299,13 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
 }
 
 export const isComputedValue = createInstanceofPredicate("ComputedValue", ComputedValue)
+
+export function computingValueStart(computingValue: ComputedValue<any> | null) {
+    const prev = globalState.computingValue
+    globalState.computingValue = computingValue
+    return prev
+}
+
+export function computingValueEnd(prev: ComputedValue<any> | null) {
+    globalState.computingValue = prev
+}

src/core/globalstate.ts

@@ -1,4 +1,4 @@
-import { IDerivation, IObservable, Reaction, die, getGlobal } from "../internal"
+import { IDerivation, IObservable, ComputedValue, Reaction, die, getGlobal } from "../internal"
 
 /**
  * These values will persist if global state is reset
@@ -135,6 +135,11 @@ export class MobXGlobals {
      * print warnings about code that would fail if proxies weren't available
      */
     verifyProxies = false
+
+    /**
+     * Currently running ComputedValue
+     */
+    computingValue: ComputedValue<any> | null = null
 }
 
 let canMergeGlobalState = true

test/v5/base/__snapshots__/action.js.snap

@@ -2,13 +2,13 @@
 
 exports[`error logging, #1836 - 1 1`] = `
 Array [
-  "<STDOUT> [mobx] (error in reaction 'Autorun@68' suppressed, fix error of causing action below)",
+  "<STDOUT> [mobx] (error in reaction 'Autorun@63' suppressed, fix error of causing action below)",
   "<STDERR> Error: Action error",
 ]
 `;
 
 exports[`error logging, #1836 - 2 1`] = `
 Array [
-  "<STDERR> [mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: 'Reaction[Autorun@71]'",
+  "<STDERR> [mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: 'Reaction[Autorun@66]'",
 ]
 `;

test/v5/base/action.js

@@ -207,31 +207,6 @@ test("should be possible to change observed state in an action called from compu
     d()
 })
 
-test("should be possible to change observed state in an action called from computed", () => {
-    const a = mobx.observable.box(2)
-    const d = mobx.autorun(() => {
-        a.get()
-    })
-
-    const testAction = mobx.action(() => {
-        a.set(3)
-    })
-
-    const c = mobx.computed(() => {
-        testAction()
-        return a.get()
-    })
-
-    expect(
-        utils.grabConsole(() => {
-            c.get()
-        })
-    ).toBe("")
-
-    mobx._resetGlobalState()
-    d()
-})
-
 test("action in autorun should be untracked", () => {
     const a = mobx.observable.box(2)
     const b = mobx.observable.box(3)
@@ -617,7 +592,7 @@ test("auto action should not update state from inside a derivation", async () =>
                 double()
             })
         ).toMatchInlineSnapshot(
-            `"<STDOUT> [MobX] Side effects like changing state are not allowed at this point. Are you trying to modify state from, for example, a computed value or the render function of a React component? You can wrap side effects in 'runInAction' (or decorate functions with 'action') if needed. Tried to modify: ObservableValue@79"`
+            `"<STDOUT> [MobX] Side effects like changing state are not allowed at this point. Are you trying to modify state from, for example, a computed value or the render function of a React component? You can wrap side effects in 'runInAction' (or decorate functions with 'action') if needed. Tried to modify: ObservableValue@74"`
         )
         return a.get() === 2
     })
@@ -648,3 +623,46 @@ test("auto action should not update state from inside a derivation", async () =>
     })
     d()
 })
+
+test("warn on calls action while computed value is computing", () => {
+    const matchWarnReg = /Don't call Action .* while ComputedValue .* is computing/
+    const square = mobx.extendObservable(
+        {},
+        {
+            width: 5,
+            get area() {
+                const retArea = this.width * this.width
+                this.changeWidth(this.width + 1)
+                return retArea
+            },
+            changeWidth(newWidth) {
+                this.width = newWidth
+            }
+        },
+        {
+            changeWidth: mobx.action
+        }
+    )
+    expect(utils.grabConsole(() => square.area)).toMatch(matchWarnReg)
+
+    class Square {
+        constructor() {
+            mobx.makeObservable(this)
+        }
+
+        @mobx.observable width = 5
+
+        @mobx.computed get area() {
+            const retArea = this.width * this.width
+            this.changeWidth(this.width + 1)
+            return retArea
+        }
+
+        @mobx.action
+        changeWidth(newWidth) {
+            this.width = newWidth
+        }
+    }
+    const squareInst = new Square()
+    expect(utils.grabConsole(() => squareInst.area)).toMatch(matchWarnReg)
+})