Reintroduce observable state,props,context in mobx-react

.changeset/bright-hornets-listen.md

@@ -0,0 +1,5 @@
+---
+"mobx-react": patch
+---
+
+Reintroduce observable state,props,context in mobx-react

packages/mobx-react/README.md

@@ -58,6 +58,8 @@ Function (and decorator) that converts a React component definition, React compo
 
 #### Class Components
 
+When using component classes, `this.props` and `this.state` will be made observables, so the component will react to all changes in props and state that are used by `render`.
+
 `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.
@@ -88,92 +90,6 @@ class TodoView extends React.Component {
 const TodoView = observer(({ todo }) => <div>{todo.title}</div>)
 ```
 
-##### Note on using props and state in derivations
-
-`mobx-react` version 6 and lower would automatically turn `this.state` and `this.props` into observables.
-This has the benefit that computed properties and reactions were able to observe those.
-However, since this pattern is fundamentally incompatible with `StrictMode` in React 18.2 and higher, this behavior has been removed in React 18.
-
-As a result, we recommend to no longer mark properties as `@computed` in observer components if they depend on `this.state` or `this.props`.
-
-```javascript
-@observer
-class Doubler extends React.Component<{ counter: number }> {
-    @computed // BROKEN! <-- @computed should be removed in mobx-react > 7
-    get doubleValue() {
-        // Changes to this.props will no longer be detected properly, to fix it,
-        // remove the @computed annotation.
-        return this.props * 2
-    }
-
-    render() {
-        return <div>{this.doubleValue}</div>
-    }
-}
-```
-
-Similarly, reactions will no longer respond to `this.state` / `this.props`. This can be overcome by creating an observable copy:
-
-```javascript
-@observer
-class Alerter extends React.Component<{ counter: number }> {
-    @observable observableCounter: number
-    reactionDisposer
-
-    constructor(props) {
-        this.observableCounter = counter
-    }
-
-    componentDidMount() {
-        // set up a reaction, by observing the observable,
-        // rather than the prop which is non-reactive:
-        this.reactionDisposer = autorun(() => {
-            if (this.observableCounter > 10) {
-                alert("Reached 10!")
-            }
-        })
-    }
-
-    componentDidUpdate() {
-        // sync the observable from props
-        this.observableCounter = this.props.counter
-    }
-
-    componentWillUnmount() {
-        this.reactionDisposer()
-    }
-
-    render() {
-        return <div>{this.props.counter}</div>
-    }
-}
-```
-
-MobX-react will try to detect cases where `this.props`, `this.state` or `this.context` are used by any other derivation than the `render` method of the owning component and throw.
-This is to make sure that neither computed properties, nor reactions, nor other components accidentally rely on those fields to be reactive.
-
-This includes cases where a render callback is passed to a child, that will read from the props or state of a parent component.
-As a result, passing a function that might later read a property of a parent in a reactive context will throw as well.
-Instead, when using a callback function that is being passed to an `observer` based child, the capture should be captured locally first:
-
-```javascript
-@observer
-class ChildWrapper extends React.Component<{ counter: number }> {
-    render() {
-        // Collapsible is an observer component that should respond to this.counter,
-        // if it is expanded
-
-        // BAD:
-        return <Collapsible onRenderContent={() => <h1>{this.props.counter}</h1>} />
-
-        // GOOD: (causes to pass down a fresh callback whenever counter changes,
-        // that doesn't depend on its parents props)
-        const counter = this.props.counter
-        return <Collapsible onRenderContent={() => <h1>{counter}</h1>} />
-    }
-}
-```
-
 ### `Observer`
 
 `Observer` is a React component, which applies `observer` to an anonymous region in your component.

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

@@ -0,0 +1,63 @@
+import React from "react"
+import { observer } from "../src"
+import { render, act } from "@testing-library/react"
+
+/**
+ *  some test suite is too tedious
+ */
+
+afterEach(() => {
+    jest.useRealTimers()
+})
+
+// let consoleWarnMock: jest.SpyInstance | undefined
+// afterEach(() => {
+//     consoleWarnMock?.mockRestore()
+// })
+
+test("TODO", async () => {
+    let renderCount = 0
+    const Child = observer(function Child({ children }) {
+        renderCount++
+        // Accesses Parent's this.props
+        return children()
+    })
+
+    @observer
+    class Parent extends React.Component<any> {
+        // intentionally stable, so test breaks when you disable observable props (comment line 239 in observerClass)
+        renderCallback = () => {
+            return this.props.x
+        }
+        render() {
+            // Access observable props as part of child
+            return <Child>{this.renderCallback}</Child>
+        }
+    }
+
+    function Root() {
+        const [x, setX] = React.useState(0)
+        // Send new props to Parent
+        return (
+            <div onClick={() => setX(x => x + 1)}>
+                <Parent x={x} />
+            </div>
+        )
+    }
+
+    const app = <Root />
+
+    const { unmount, container } = render(app)
+
+    expect(container).toHaveTextContent("0")
+    expect(renderCount).toBe(1)
+
+    await new Promise(resolve => setTimeout(() => resolve(null), 1000))
+    act(() => {
+        console.log("changing state")
+        container.querySelector("div")?.click()
+    })
+    expect(container).toHaveTextContent("1")
+    expect(renderCount).toBe(2)
+    unmount()
+})

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

@@ -212,6 +212,113 @@ test("verify prop changes are picked up", () => {
     expect(container.textContent).toMatchInlineSnapshot(`"1.2.test.0"`)
 })
 
+test("verify props is reactive", () => {
+    function createItem(subid, label) {
+        const res = observable(
+            {
+                subid,
+                id: 1,
+                label: label,
+                get text() {
+                    events.push(["compute", this.subid])
+                    return (
+                        this.id +
+                        "." +
+                        this.subid +
+                        "." +
+                        this.label +
+                        "." +
+                        data.items.indexOf(this as any)
+                    )
+                }
+            },
+            {},
+            { proxy: false }
+        )
+        res.subid = subid // non reactive
+        return res
+    }
+
+    const data = observable({
+        items: [createItem(1, "hi")]
+    })
+    const events: Array<any> = []
+
+    class Child extends React.Component<any, any> {
+        constructor(p) {
+            super(p)
+            makeObservable(this)
+        }
+
+        @computed
+        get computedLabel() {
+            events.push(["computed label", this.props.item.subid])
+            return this.props.item.label
+        }
+        componentDidMount() {
+            events.push(["mount"])
+        }
+        componentDidUpdate(prevProps) {
+            events.push(["update", prevProps.item.subid, this.props.item.subid])
+        }
+        render() {
+            events.push(["render", this.props.item.subid, this.props.item.text, this.computedLabel])
+            return (
+                <span>
+                    {this.props.item.text}
+                    {this.computedLabel}
+                </span>
+            )
+        }
+    }
+
+    const ChildAsObserver = observer(Child)
+
+    const Parent = observer(
+        class Parent extends React.Component<any, any> {
+            render() {
+                return (
+                    <div onClick={changeStuff.bind(this)} id="testDiv">
+                        {data.items.map(item => (
+                            <ChildAsObserver key="fixed" item={item} />
+                        ))}
+                    </div>
+                )
+            }
+        }
+    )
+
+    const Wrapper = () => <Parent />
+
+    function changeStuff() {
+        act(() => {
+            transaction(() => {
+                // components start rendeirng a new item, but computed is still based on old value
+                data.items = [createItem(2, "test")]
+            })
+        })
+    }
+
+    const { container } = render(<Wrapper />)
+    expect(events.sort()).toEqual(
+        [["mount"], ["compute", 1], ["computed label", 1], ["render", 1, "1.1.hi.0", "hi"]].sort()
+    )
+
+    events.splice(0)
+    let testDiv = container.querySelector("#testDiv") as HTMLElement
+    testDiv.click()
+
+    expect(events.sort()).toEqual(
+        [
+            ["compute", 1],
+            ["update", 1, 2],
+            ["compute", 2],
+            ["computed label", 2],
+            ["render", 2, "1.2.test.0", "test"]
+        ].sort()
+    )
+})
+
 test("no re-render for shallow equal props", async () => {
     function createItem(subid, label) {
         const res = observable({
@@ -308,3 +415,40 @@ test("lifecycle callbacks called with correct arguments", () => {
     let testButton = container.querySelector("#testButton") as HTMLElement
     testButton.click()
 })
+
+test("verify props are reactive in constructor", () => {
+    const propValues: Array<any> = []
+    let constructorCallsCount = 0
+
+    const Component = observer(
+        class Component extends React.Component<any, any> {
+            disposer: IReactionDisposer
+            constructor(props, context) {
+                super(props, context)
+                constructorCallsCount++
+                this.disposer = reaction(
+                    () => this.props.prop,
+                    prop => propValues.push(prop),
+                    {
+                        fireImmediately: true
+                    }
+                )
+            }
+
+            componentWillUnmount() {
+                this.disposer()
+            }
+
+            render() {
+                return <div />
+            }
+        }
+    )
+
+    const { rerender } = render(<Component prop="1" />)
+    rerender(<Component prop="2" />)
+    rerender(<Component prop="3" />)
+    rerender(<Component prop="4" />)
+    expect(constructorCallsCount).toEqual(1)
+    expect(propValues).toEqual(["1", "2", "3", "4"])
+})