Merge pull request #496 from megaplan/issue_478_observale_props_in_co…

…nstructor

Make props & state observable in constructor & componentWillMount

src/observer.js

@@ -20,6 +20,37 @@ let warnedAboutObserverInjectDeprecation = false
 export const componentByNodeRegistry = typeof WeakMap !== "undefined" ? new WeakMap() : undefined
 export const renderReporter = new EventEmitter()
 
+const knownNonEnumerablePropsKey = Symbol("knownNonEnumerableProps")
+const skipRenderKey = Symbol("skipRender")
+const isForcingUpdateKey = Symbol("isForcingUpdate")
+
+/**
+ * Helper to set `prop` to `this` as non-enumerable (hidden prop)
+ * @param prop
+ * @param value
+ */
+function setHiddenProp(target, prop, value) {
+    if (!target[knownNonEnumerablePropsKey]) {
+        Object.defineProperty(target, knownNonEnumerablePropsKey, {
+            enumerable: false,
+            configurable: false,
+            writable: false,
+            value: {}
+        })
+    }
+    if (!target[knownNonEnumerablePropsKey][prop]) {
+        Object.defineProperty(target, prop, {
+            enumerable: false,
+            configurable: false,
+            writable: true,
+            value: value
+        })
+        target[knownNonEnumerablePropsKey][prop] = true
+    } else {
+        target[prop] = value
+    }
+}
+
 function findDOMNode(component) {
     if (baseFindDOMNode) {
         try {
@@ -116,29 +147,6 @@ function is(x, y) {
 function makeComponentReactive(render) {
     if (isUsingStaticRendering === true) return render.call(this)
 
-    function makePropertyObservableReference(propName) {
-        let valueHolder = this[propName]
-        const atom = createAtom("reactive " + propName)
-        Object.defineProperty(this, propName, {
-            configurable: true,
-            enumerable: true,
-            get: function() {
-                atom.reportObserved()
-                return valueHolder
-            },
-            set: function set(v) {
-                if (!isForcingUpdate && !shallowEqual(valueHolder, v)) {
-                    valueHolder = v
-                    skipRender = true
-                    atom.reportChanged()
-                    skipRender = false
-                } else {
-                    valueHolder = v
-                }
-            }
-        })
-    }
-
     function reactiveRender() {
         isRenderingPending = false
         let exception = undefined
@@ -177,17 +185,12 @@ function makeComponentReactive(render) {
      * If props are shallowly modified, react will render anyway,
      * so atom.reportChanged() should not result in yet another re-render
      */
-    let skipRender = false
+    setHiddenProp(this, skipRenderKey, false)
     /**
      * forceUpdate will re-assign this.props. We don't want that to cause a loop,
      * so detect these changes
      */
-    let isForcingUpdate = false
-
-    // make this.props an observable reference, see #124
-    makePropertyObservableReference.call(this, "props")
-    // make state an observable reference
-    makePropertyObservableReference.call(this, "state")
+    setHiddenProp(this, isForcingUpdateKey, false)
 
     // wire up reactive render
     const baseRender = render.bind(this)
@@ -206,11 +209,11 @@ function makeComponentReactive(render) {
                 // However, people also claim this migth happen during unit tests..
                 let hasError = true
                 try {
-                    isForcingUpdate = true
-                    if (!skipRender) Component.prototype.forceUpdate.call(this)
+                    setHiddenProp(this, isForcingUpdateKey, true)
+                    if (!this[skipRenderKey]) Component.prototype.forceUpdate.call(this)
                     hasError = false
                 } finally {
-                    isForcingUpdate = false
+                    setHiddenProp(this, isForcingUpdateKey, false)
                     if (hasError) reaction.dispose()
                 }
             }
@@ -273,6 +276,35 @@ const reactiveMixin = {
     }
 }
 
+function makeObservableProp(target, propName) {
+    const valueHolderKey = Symbol(propName + " value holder")
+    const atomHolderKey = Symbol(propName + " atom holder")
+    function getAtom() {
+        if (!this[atomHolderKey]) {
+            setHiddenProp(this, atomHolderKey, createAtom("reactive " + propName))
+        }
+        return this[atomHolderKey]
+    }
+    Object.defineProperty(target, propName, {
+        configurable: true,
+        enumerable: true,
+        get: function() {
+            getAtom.call(this).reportObserved()
+            return this[valueHolderKey]
+        },
+        set: function set(v) {
+            if (!this[isForcingUpdateKey] && !shallowEqual(this[valueHolderKey], v)) {
+                setHiddenProp(this, valueHolderKey, v)
+                setHiddenProp(this, skipRenderKey, true)
+                getAtom.call(this).reportChanged()
+                setHiddenProp(this, skipRenderKey, false)
+            } else {
+                setHiddenProp(this, valueHolderKey, v)
+            }
+        }
+    })
+}
+
 /**
  * Observer function / decorator
  */
@@ -340,6 +372,8 @@ export function observer(arg1, arg2) {
     const target = componentClass.prototype || componentClass
     mixinLifecycleEvents(target)
     componentClass.isMobXReactObserver = true
+    makeObservableProp(target, "props")
+    makeObservableProp(target, "state")
     const baseRender = target.render
     target.render = function() {
         return makeComponentReactive.call(this, baseRender)

test/issue21.test.js

@@ -449,3 +449,55 @@ test("lifecycle callbacks called with correct arguments", async () => {
     await asyncReactDOMRender(<Root />, testRoot)
     testRoot.querySelector("#testButton").click()
 })
+
+test("verify props are reactive in componentWillMount and constructor", async () => {
+    const prop1Values = []
+    const prop2Values = []
+    let componentWillMountCallsCount = 0
+    let constructorCallsCount = 0
+
+    const Component = observer(
+        class Component extends React.Component {
+            constructor(props, context) {
+                super(props, context)
+                constructorCallsCount++
+                this.disposer1 = mobx.reaction(
+                    () => this.props.prop1,
+                    prop => prop1Values.push(prop),
+                    {
+                        fireImmediately: true
+                    }
+                )
+            }
+
+            componentWillMount() {
+                componentWillMountCallsCount++
+                this.disposer2 = mobx.reaction(
+                    () => this.props.prop2,
+                    prop => prop2Values.push(prop),
+                    {
+                        fireImmediately: true
+                    }
+                )
+            }
+
+            componentWillUnmount() {
+                this.disposer1()
+                this.disposer2()
+            }
+
+            render() {
+                return <div />
+            }
+        }
+    )
+
+    await asyncReactDOMRender(<Component prop1="1" prop2="4" />, testRoot)
+    await asyncReactDOMRender(<Component prop1="2" prop2="3" />, testRoot)
+    await asyncReactDOMRender(<Component prop1="3" prop2="2" />, testRoot)
+    await asyncReactDOMRender(<Component prop1="4" prop2="1" />, testRoot)
+    expect(constructorCallsCount).toEqual(1)
+    expect(componentWillMountCallsCount).toEqual(1)
+    expect(prop1Values).toEqual(["1", "2", "3", "4"])
+    expect(prop2Values).toEqual(["4", "3", "2", "1"])
+})