Fix #3648: observableRequiresReaction/computedRequiresReaction shouldn't warn with enableStaticRendering(true)

.changeset/fluffy-hairs-study.md

@@ -0,0 +1,6 @@
+---
+"mobx-react": patch
+"mobx-react-lite": patch
+---
+
+fix #3648: `observableRequiresReaction`/`computedRequiresReaction` shouldn't warn with `enableStaticRendering(true)`

.changeset/shaggy-carpets-move.md

@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+`computedRequiresReaction` respects `globalState.allowStateReads`

packages/mobx-react-lite/__tests__/__snapshots__/observer.test.tsx.snap

@@ -16,6 +16,8 @@ exports[`changing state in render should fail 2`] = `
 </div>
 `;
 
+exports[`#3648 enableStaticRendering doesn't warn with observableRequiresReaction/computedRequiresReaction 1`] = `[MockFunction]`;
+
 exports[`issue 12 init state is correct 1`] = `
 <div>
   <div>

packages/mobx-react-lite/__tests__/observer.test.tsx

@@ -1084,3 +1084,24 @@ test("Anonymous component displayName #3192", () => {
     expect(observerError.message).toEqual(memoError.message)
     consoleErrorSpy.mockRestore()
 })
+
+test("#3648 enableStaticRendering doesn't warn with observableRequiresReaction/computedRequiresReaction", () => {
+    consoleWarnMock = jest.spyOn(console, "warn").mockImplementation(() => {})
+    try {
+        enableStaticRendering(true)
+        mobx.configure({ observableRequiresReaction: true, computedRequiresReaction: true })
+        const o = mobx.observable.box(0, { name: "o" })
+        const c = mobx.computed(() => o.get(), { name: "c" })
+        const TestCmp = observer(() => <div>{o.get() + c.get()}</div>)
+
+        const { unmount, container } = render(<TestCmp />)
+        expect(container).toHaveTextContent("0")
+        unmount()
+
+        expect(consoleWarnMock).toMatchSnapshot()
+    } finally {
+        enableStaticRendering(false)
+        mobx._resetGlobalState()
+        consoleWarnMock.mockRestore()
+    }
+})

packages/mobx-react-lite/src/observer.ts

@@ -1,6 +1,4 @@
 import { forwardRef, memo } from "react"
-
-import { isUsingStaticRendering } from "./staticRendering"
 import { useObserver } from "./useObserver"
 
 let warnObserverOptionsDeprecated = true
@@ -79,10 +77,6 @@ export function observer<P extends object, TRef = {}>(
     }
 
     // The working of observer is explained step by step in this talk: https://www.youtube.com/watch?v=cPF4iBedoF0&feature=youtu.be&t=1307
-    if (isUsingStaticRendering()) {
-        return baseComponent
-    }
-
     let useForwardRef = options?.forwardRef ?? false
     let render = baseComponent
 

packages/mobx-react-lite/src/useObserver.ts

@@ -1,4 +1,4 @@
-import { Reaction } from "mobx"
+import { Reaction, _allowStateReadsEnd, _allowStateReadsStart } from "mobx"
 import React from "react"
 import { printDebugValue } from "./utils/printDebugValue"
 import { observerFinalizationRegistry } from "./utils/observerFinalizationRegistry"
@@ -36,7 +36,12 @@ function objectToBeRetainedByReactFactory() {
 
 export function useObserver<T>(fn: () => T, baseComponentName: string = "observed"): T {
     if (isUsingStaticRendering()) {
-        return fn()
+        let allowStateReads = _allowStateReadsStart?.(true)
+        try {
+            return fn()
+        } finally {
+            _allowStateReadsEnd?.(allowStateReads)
+        }
     }
 
     const [objectRetainedByReact] = React.useState(objectToBeRetainedByReactFactory)

packages/mobx-react/__tests__/__snapshots__/observer.test.tsx.snap

@@ -18,6 +18,8 @@ exports[`#797 - replacing this.render should trigger a warning 1`] = `
 }
 `;
 
+exports[`#3648 enableStaticRendering doesn't warn with observableRequiresReaction/computedRequiresReaction 1`] = `[MockFunction]`;
+
 exports[`Redeclaring an existing observer component as an observer should log a warning 1`] = `
 [MockFunction] {
   "calls": Array [

packages/mobx-react/__tests__/observer.test.tsx

@@ -8,7 +8,8 @@ import {
     computed,
     observable,
     transaction,
-    makeObservable
+    makeObservable,
+    configure
 } from "mobx"
 import { withConsole } from "./utils/withConsole"
 /**
@@ -1037,3 +1038,30 @@ test("SSR works #3448", () => {
 
     expect(consoleWarnMock).toMatchSnapshot()
 })
+
+test("#3648 enableStaticRendering doesn't warn with observableRequiresReaction/computedRequiresReaction", () => {
+    consoleWarnMock = jest.spyOn(console, "warn").mockImplementation(() => {})
+    try {
+        enableStaticRendering(true)
+        configure({ observableRequiresReaction: true, computedRequiresReaction: true })
+        const o = observable.box(0, { name: "o" })
+        const c = computed(() => o.get(), { name: "c" })
+
+        @observer
+        class TestCmp extends React.Component<any> {
+            render() {
+                return o.get() + c.get()
+            }
+        }
+
+        const { unmount, container } = render(<TestCmp />)
+        expect(container).toHaveTextContent("0")
+        unmount()
+
+        expect(consoleWarnMock).toMatchSnapshot()
+    } finally {
+        enableStaticRendering(false)
+        _resetGlobalState()
+        consoleWarnMock.mockRestore()
+    }
+})

packages/mobx-react/src/observerClass.ts

@@ -67,7 +67,14 @@ export function makeClassComponentObserver(
     }
     target.render = function () {
         this.render = isUsingStaticRendering()
-            ? originalRender
+            ? function () {
+                  let allowStateReads = _allowStateReadsStart?.(true)
+                  try {
+                      return originalRender()
+                  } finally {
+                      _allowStateReadsEnd?.(allowStateReads)
+                  }
+              }
             : createReactiveRender.call(this, originalRender)
         return this.render()
     }

packages/mobx/src/api/configure.ts

@@ -53,7 +53,6 @@ export function configure(options: {
             globalState[key] = !!options[key]
         }
     })
-    globalState.allowStateReads = !globalState.observableRequiresReaction
     if (__DEV__ && globalState.disableErrorBoundaries === true) {
         console.warn(
             "WARNING: Debug feature only. MobX will NOT recover from errors when `disableErrorBoundaries` is enabled."

packages/mobx/src/core/computedvalue.ts

@@ -309,9 +309,10 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
             )
         }
         if (
-            typeof this.requiresReaction_ === "boolean"
+            !globalState.allowStateReads &&
+            (typeof this.requiresReaction_ === "boolean"
                 ? this.requiresReaction_
-                : globalState.computedRequiresReaction
+                : globalState.computedRequiresReaction)
         ) {
             console.warn(
                 `[mobx] Computed value '${this.name_}' is being read outside a reactive context. Doing a full recompute.`

packages/mobx/src/core/globalstate.ts

@@ -90,9 +90,9 @@ export class MobXGlobals {
 
     /**
      * Is it allowed to read observables at this point?
-     * Used to hold the state needed for `observableRequiresReaction`
+     * Used to hold the state needed for `observableRequiresReaction`/`computedRequiresReaction`
      */
-    allowStateReads = true
+    allowStateReads = false
 
     /**
      * If strict mode is enabled, state changes are by default not allowed