Fix #3315: requiresObservable always takes precedence over global reactionRequiresObservable

.changeset/stale-melons-approve.md

@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+`requiresObservable` always takes precedence over global `reactionRequiresObservable`

docs/reactions.md

@@ -23,7 +23,7 @@ Beyond that, there are also [`reaction`](#reaction) and [`when`](#when).
 
 Usage:
 
--   `autorun(effect: (reaction) => void)`
+-   `autorun(effect: (reaction) => void, options?)`
 
 The `autorun` function accepts one function that should run every time anything it observes changes.
 It also runs once when you create the `autorun` itself. It only responds to changes in observable state, things you have annotated `observable` or `computed`.

packages/mobx/__tests__/v5/base/observables.js

@@ -17,7 +17,7 @@ const {
 const utils = require("../../v5/utils/test-utils")
 const { MAX_SPLICE_SIZE } = require("../../../src/internal")
 
-const voidObserver = function () { }
+const voidObserver = function () {}
 
 function buffer() {
     const b = []
@@ -2100,7 +2100,7 @@ test("extendObservable should not accept complex objects as second argument", ()
 })
 
 test("observable ignores class instances #2579", () => {
-    class C { }
+    class C {}
     const c = new C()
     expect(observable(c)).toBe(c)
 })
@@ -2121,9 +2121,9 @@ test("configure({ safeDescriptors: false })", () => {
 
     class Clazz {
         observable = 0
-        action() { }
-        get computed() { }
-        *flow() { }
+        action() {}
+        get computed() {}
+        *flow() {}
         constructor() {
             mobx.makeObservable(this, {
                 observable: mobx.observable,
@@ -2139,9 +2139,9 @@ test("configure({ safeDescriptors: false })", () => {
 
     const plain = mobx.observable({
         observable: 0,
-        action() { },
-        get computed() { },
-        *flow() { }
+        action() {},
+        get computed() {},
+        *flow() {}
     })
 
     checkDescriptors(plain)
@@ -2251,34 +2251,69 @@ test("ObservableArray.splice", () => {
 })
 
 describe("`requiresReaction` takes precedence over global `computedRequiresReaction`", () => {
-    let warnMsg = "[mobx] Computed value 'TestComputed' is being read outside a reactive context. Doing a full recompute.";
-    let consoleWarnSpy;
+    const name = "TestComputed"
+    let warnMsg = `[mobx] Computed value '${name}' is being read outside a reactive context. Doing a full recompute.`
+    let consoleWarnSpy
     beforeEach(() => {
         consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation()
     })
     afterEach(() => {
         consoleWarnSpy.mockRestore()
-        mobx._resetGlobalState();
+        mobx._resetGlobalState()
     })
 
-    test('`undefined`', () => {
+    test("`undefined`", () => {
         mobx.configure({ computedRequiresReaction: true })
-        const c = mobx.computed(() => { }, { name: 'TestComputed' });
-        c.get();
-        expect(consoleWarnSpy).toHaveBeenLastCalledWith(warnMsg);
+        const c = mobx.computed(() => {}, { name })
+        c.get()
+        expect(consoleWarnSpy).toHaveBeenLastCalledWith(warnMsg)
     })
 
-    test('`true` over `false`', () => {
+    test("`true` over `false`", () => {
         mobx.configure({ computedRequiresReaction: false })
-        const c = mobx.computed(() => { }, { name: 'TestComputed', requiresReaction: true });
-        c.get();
-        expect(consoleWarnSpy).toHaveBeenLastCalledWith(warnMsg);
+        const c = mobx.computed(() => {}, { name, requiresReaction: true })
+        c.get()
+        expect(consoleWarnSpy).toHaveBeenLastCalledWith(warnMsg)
     })
 
-    test('`false` over `true`', () => {
+    test("`false` over `true`", () => {
         mobx.configure({ computedRequiresReaction: true })
-        const c = mobx.computed(() => { }, { name: 'TestComputed', requiresReaction: false });
-        c.get();
-        expect(consoleWarnSpy).not.toHaveBeenCalled();
+        const c = mobx.computed(() => {}, { name, requiresReaction: false })
+        c.get()
+        expect(consoleWarnSpy).not.toHaveBeenCalled()
+    })
+})
+
+describe("`requiresObservable` takes precedence over global `reactionRequiresObservable`", () => {
+    const name = "TestReaction"
+    let warnMsg = `[mobx] Derivation '${name}' is created/updated without reading any observable value.`
+    let consoleWarnSpy
+    beforeEach(() => {
+        consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation()
+    })
+    afterEach(() => {
+        consoleWarnSpy.mockRestore()
+        mobx._resetGlobalState()
+    })
+
+    test("`undefined`", () => {
+        mobx.configure({ reactionRequiresObservable: true })
+        const dispose = mobx.autorun(() => {}, { name })
+        dispose()
+        expect(consoleWarnSpy).toHaveBeenLastCalledWith(warnMsg)
+    })
+
+    test("`true` over `false`", () => {
+        mobx.configure({ reactionRequiresObservable: false })
+        const dispose = mobx.autorun(() => {}, { name, requiresObservable: true })
+        dispose()
+        expect(consoleWarnSpy).toHaveBeenLastCalledWith(warnMsg)
+    })
+
+    test("`false` over `true`", () => {
+        mobx.configure({ reactionRequiresObservable: true })
+        const dispose = mobx.autorun(() => {}, { name, requiresObservable: false })
+        dispose()
+        expect(consoleWarnSpy).not.toHaveBeenCalled()
     })
 })

packages/mobx/src/core/derivation.ts

@@ -203,7 +203,11 @@ function warnAboutDerivationWithoutDependencies(derivation: IDerivation) {
         return
     }
 
-    if (globalState.reactionRequiresObservable || derivation.requiresObservable_) {
+    if (
+        typeof derivation.requiresObservable_ === "boolean"
+            ? derivation.requiresObservable_
+            : globalState.reactionRequiresObservable
+    ) {
         console.warn(
             `[mobx] Derivation '${derivation.name_}' is created/updated without reading any observable value.`
         )

packages/mobx/src/core/reaction.ts

@@ -67,7 +67,7 @@ export class Reaction implements IDerivation, IReactionPublic {
         public name_: string = __DEV__ ? "Reaction@" + getNextId() : "Reaction",
         private onInvalidate_: () => void,
         private errorHandler_?: (error: any, derivation: IDerivation) => void,
-        public requiresObservable_ = false
+        public requiresObservable_?
     ) {}
 
     onBecomeStale_() {
@@ -169,7 +169,7 @@ export class Reaction implements IDerivation, IReactionPublic {
         if (!globalState.suppressReactionErrors) {
             console.error(message, error)
             /** If debugging brought you here, please, read the above message :-). Tnx! */
-        } else if (__DEV__) {console.warn(`[mobx] (error in reaction '${this.name_}' suppressed, fix error of causing action below)`)} // prettier-ignore
+        } else if (__DEV__) { console.warn(`[mobx] (error in reaction '${this.name_}' suppressed, fix error of causing action below)`) } // prettier-ignore
 
         if (__DEV__ && isSpyEnabled()) {
             spyReport({