wip

.changeset/flat-pumas-cross.md

@@ -3,3 +3,7 @@
 ---
 
 fix #3671: `observer` + `StrictMode`
+BC: Class component's `props`/`state`/`context` are no longer observable. Attempt to use these in any derivation other than component's `render` throws and error.
+BC: Attempt to apply `observer` on a component that is already an `observer` throws instead of warning.
+docs: link for mobx-react docs
+docs: extending `observer` class components is not supported

docs/react-integration.md

@@ -349,7 +349,7 @@ const TimerView = observer(
 )
 ```
 
-Check out [mobx-react docs](https://github.com/mobxjs/mobx-react#api-documentation) for more information.
+Check out [mobx-react docs](https://github.com/mobxjs/mobx/tree/main/packages/mobx-react#class-components) for more information.
 
 </details>
 

packages/mobx-react/README.md

@@ -61,6 +61,8 @@ When using component classes, `this.props` and `this.state` will be made observa
 
 `shouldComponentUpdate` is not supported. As such, it is recommended that class components extend `React.PureComponent`. The `observer` will automatically patch non-pure class components with an internal implementation of `React.PureComponent` if necessary.
 
+Extending `observer` class components is not supported. Always apply `observer` only on the last class in the inheritance chain.
+
 See the [MobX](https://mobx.js.org/react-integration.html#react-integration) documentation for more details.
 
 ```javascript

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

@@ -2,22 +2,7 @@
 
 exports[`#3492 should not cause warning by calling forceUpdate on uncommited components 1`] = `[MockFunction]`;
 
-exports[`Redeclaring an existing observer component as an observer should log a warning 1`] = `
-[MockFunction] {
-  "calls": Array [
-    Array [
-      "The provided component class (AlreadyObserver)
-                has already been declared as an observer component.",
-    ],
-  ],
-  "results": Array [
-    Object {
-      "type": "return",
-      "value": undefined,
-    },
-  ],
-}
-`;
+exports[`Redeclaring an existing observer component as an observer should throw 1`] = `"The provided component class (AlreadyObserver) has already been declared as an observer component."`;
 
 exports[`SSR works #3448 1`] = `[MockFunction]`;
 

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

@@ -877,18 +877,15 @@ test.skip("#709 - applying observer on React.memo component", () => {
     render(<Observed />, { wrapper: ErrorCatcher })
 })
 
-test("Redeclaring an existing observer component as an observer should log a warning", () => {
-    consoleWarnMock = jest.spyOn(console, "warn").mockImplementation(() => {})
-
+test("Redeclaring an existing observer component as an observer should throw", () => {
     @observer
     class AlreadyObserver extends React.Component<any, any> {
         render() {
             return <div />
         }
     }
 
-    observer(AlreadyObserver)
-    expect(consoleWarnMock).toMatchSnapshot()
+    expect(() => observer(AlreadyObserver)).toThrowErrorMatchingSnapshot()
 })
 
 test("Missing render should throw", () => {
@@ -1118,7 +1115,6 @@ test(`Observable changes in componenWillUnmount don't cause any warnings or erro
     consoleWarnSpy.mockRestore()
 })
 
-// TODO
 test(`Observable prop workaround`, () => {
     configure({
         enforceActions: "observed"
@@ -1185,23 +1181,26 @@ test(`Observable prop workaround`, () => {
     unmount()
 })
 
-// TODO
-test.skip(`Observable props/state/context workaround`, () => {
+test(`Observable props/state/context workaround`, () => {
     configure({
         enforceActions: "observed"
     })
 
-    const propValues: Array<string> = []
+    const reactionResults: Array<string> = []
+
+    const ContextType = React.createContext(0)
 
     const TestCmp = observer(
-        class TestCmp extends React.Component<any> {
+        class TestCmp extends React.Component<any, any> {
+            static contextType = ContextType
+
             disposeReaction: IReactionDisposer | undefined
             observableProps: any
             observableState: any
             observableContext: any
 
-            constructor(props) {
-                super(props)
+            constructor(props, context) {
+                super(props, context)
                 this.state = {
                     x: 0
                 }
@@ -1224,7 +1223,7 @@ test.skip(`Observable props/state/context workaround`, () => {
             componentDidMount(): void {
                 this.disposeReaction = reaction(
                     () => this.computed,
-                    prop => propValues.push(prop),
+                    prop => reactionResults.push(prop),
                     {
                         fireImmediately: true
                     }
@@ -1249,15 +1248,47 @@ test.skip(`Observable props/state/context workaround`, () => {
             }
 
             render() {
-                return this.computed
+                return (
+                    <span
+                        id="updateState"
+                        onClick={() => this.setState(state => ({ x: state.x + 1 }))}
+                    >
+                        {this.computed}
+                    </span>
+                )
             }
         }
     )
 
-    const { unmount, rerender } = render(<TestCmp prop={1} />)
-    // rerender(<TestCmp prop={2} />)
-    // rerender(<TestCmp prop={3} />)
-    // rerender(<TestCmp prop={4} />)
-    // expect(propValues).toEqual([1,2,3,4])
+    const App = () => {
+        const [context, setContext] = React.useState(0)
+        const [prop, setProp] = React.useState(0)
+        return (
+            <ContextType.Provider value={context}>
+                <span id="updateContext" onClick={() => setContext(val => val + 1)}></span>
+                <span id="updateProp" onClick={() => setProp(val => val + 1)}></span>
+                <TestCmp x={prop}></TestCmp>
+            </ContextType.Provider>
+        )
+    }
+
+    const { container, unmount } = render(<App />)
+
+    const updateProp = () =>
+        act(() => (container.querySelector("#updateProp") as HTMLElement).click())
+    const updateState = () =>
+        act(() => (container.querySelector("#updateState") as HTMLElement).click())
+    const updateContext = () =>
+        act(() => (container.querySelector("#updateContext") as HTMLElement).click())
+
+    expect(container).toHaveTextContent("000")
+    updateProp()
+    expect(container).toHaveTextContent("100")
+    updateState()
+    expect(container).toHaveTextContent("110")
+    updateContext()
+    expect(container).toHaveTextContent("111")
+
+    expect(reactionResults).toEqual(["000", "100", "110", "111"])
     unmount()
 })

packages/mobx-react/src/observerClass.ts

@@ -57,9 +57,8 @@ export function makeClassComponentObserver(
 
     if (componentClass[isMobXReactObserverSymbol]) {
         const displayName = getDisplayName(componentClass)
-        console.warn(
-            `The provided component class (${displayName})
-                has already been declared as an observer component.`
+        throw new Error(
+            `The provided component class (${displayName}) has already been declared as an observer component.`
         )
     } else {
         componentClass[isMobXReactObserverSymbol] = true