Observable props of observer-ed component

package.json

@@ -53,4 +53,4 @@
     "reactjs",
     "reactive"
   ]
-}
+}

src/observer.js

@@ -3,6 +3,7 @@ import React from 'react';
 import ReactDOM from 'react-dom';
 import EventEmitter from './utils/EventEmitter';
 import inject from './inject';
+import debounce from "lodash.debounce";
 
 /**
  * dev tool support
@@ -57,6 +58,33 @@ function patch(target, funcName) {
   }
 }
 
+function isObjectShallowModified(prev, next) {
+  if (null == prev || null == next || typeof prev !== "object" || typeof next !== "object") {
+    return prev !== next;
+  }
+  const keys = Object.keys(prev);
+  if (keys.length !== Object.keys(next).length) {
+    return true;
+  }
+  let key;
+  for (let i = keys.length - 1; i >= 0, key = keys[i]; i--) {
+    const newValue = next[key];
+    if (newValue !== prev[key]) {
+      return true;
+    } else if (newValue && typeof newValue === "object" && !mobx.isObservable(newValue)) {
+      /**
+       * If the newValue is still the same object, but that object is not observable,
+       * fallback to the default React behavior: update, because the object *might* have changed.
+       * If you need the non default behavior, just use the React pure render mixin, as that one
+       * will work fine with mobx as well, instead of the default implementation of
+       * observer.
+       */
+      return true;
+    }
+  }
+  return false;
+}
+
 /**
  * ReactiveMixin
  */
@@ -68,6 +96,37 @@ const reactiveMixin = {
       || (this.constructor && (this.constructor.displayName || this.constructor.name))
       || "<component>";
     const rootNodeID = this._reactInternalInstance && this._reactInternalInstance._rootNodeID;
+
+    let skipRender = false;
+
+    function makePropertyObservableReference(propName) {
+      let valueHolder = this[propName];
+      const atom = new mobx.Atom("reactive " + propName);
+      Object.defineProperty(this, propName, {
+          configurable: true, enumerable: true,
+          get: function() {
+            atom.reportObserved();
+            return valueHolder;
+          },
+          set: function set(v) {
+            if (isObjectShallowModified(valueHolder, v)) {
+              valueHolder = v;
+              skipRender = true;
+              atom.reportChanged();
+              skipRender = false;
+            } else {
+              valueHolder = v;
+            }
+          }
+      })
+    }
+
+    // make this.props an observable reference, see #124
+    makePropertyObservableReference.call(this, "props")
+    // make state an observable reference
+    makePropertyObservableReference.call(this, "state")
+
+    // wire up reactive render
     const baseRender = this.render.bind(this);
     let reaction = null;
     let isRenderingPending = false;
@@ -85,7 +144,9 @@ const reactiveMixin = {
             // If we are unmounted at this point, componentWillReact() had a side effect causing the component to unmounted
             // TODO: remove this check? Then react will properly warn about the fact that this should not happen? See #73
             // However, people also claim this migth happen during unit tests..
-            React.Component.prototype.forceUpdate.call(this)
+            if (!skipRender) {
+              React.Component.prototype.forceUpdate.call(this)
+            }
           }
         }
       });
@@ -146,27 +207,7 @@ const reactiveMixin = {
       return true;
     }
     // update if props are shallowly not equal, inspired by PureRenderMixin
-    const keys = Object.keys(this.props);
-    if (keys.length !== Object.keys(nextProps).length) {
-      return true;
-    }
-    let key;
-    for (let i = keys.length - 1; i >= 0, key = keys[i]; i--) {
-      const newValue = nextProps[key];
-      if (newValue !== this.props[key]) {
-        return true;
-      } else if (newValue && typeof newValue === "object" && !mobx.isObservable(newValue)) {
-        /**
-         * If the newValue is still the same object, but that object is not observable,
-         * fallback to the default React behavior: update, because the object *might* have changed.
-         * If you need the non default behavior, just use the React pure render mixin, as that one
-         * will work fine with mobx as well, instead of the default implementation of
-         * observer.
-         */
-        return true;
-      }
-    }
-    return false;
+    return isObjectShallowModified(this.props, nextProps);
   }
 };
 

test/issue21.js

@@ -142,10 +142,7 @@ test('verify prop changes are picked up', function(t) {
     var data = mobx.observable({
         items: [createItem(1, "hi")]
     })
-
-    var setState;
     var events = []
-    window.xxx = events
 
     var Child = observer(React.createClass({
         componentWillReceiveProps: function (nextProps) {
@@ -212,4 +209,244 @@ test('verify prop changes are picked up', function(t) {
             t.end()
         }, 100)
     })
-})
+})
+
+
+test('verify props is reactive', function(t) {
+    function createItem(subid, label) {
+        const res = mobx.observable({
+            id: 1,
+            label: label,
+            get text() {
+                events.push(["compute", this.subid])
+                return this.id + "." + this.subid + "." + this.label + "." + data.items.indexOf(this)
+            }
+        })
+        res.subid = subid // non reactive
+        return res
+    }
+
+    var data = mobx.observable({
+        items: [createItem(1, "hi")]
+    })
+    var events = []
+
+    var Child = observer(React.createClass({
+        componentWillMount() {
+            events.push(["mount"])
+            mobx.extendObservable(this, {
+                computedLabel: function() {
+                    events.push(["computed label", this.props.item.subid])
+                    return this.props.item.label
+                }
+            })
+        },
+
+        componentWillReceiveProps: function (nextProps) {
+            events.push(["receive", this.props.item.subid, nextProps.item.subid])
+        },
+
+        componentWillUpdate: function (nextProps) {
+            events.push(["update", this.props.item.subid, nextProps.item.subid])
+        },
+
+        componentWillReact: function() {
+            events.push(["react", this.props.item.subid])
+        },
+
+        render: function() {
+            events.push(["render", this.props.item.subid, this.props.item.text, this.computedLabel])
+            return React.createElement("span", {}, this.props.item.text + this.computedLabel)
+        }
+    }))
+
+    var Parent = observer(React.createClass({
+        render: function() {
+            return React.createElement("div", {
+                onClick: changeStuff.bind(this), // event is needed to get batching!
+                id: "testDiv"
+            }, data.items.map(function(item) {
+                return React.createElement(Child, {
+                    key: "fixed",
+                    item: item
+                })
+            }))
+        }
+    }))
+
+    var Wrapper = React.createClass({ render: function() {
+        return React.createElement(Parent, {})
+    }})
+
+    function changeStuff() {
+        mobx.transaction(function() {
+            // components start rendeirng a new item, but computed is still based on old value
+            data.items = [createItem(2, "test")]
+        })
+    }
+
+    ReactDOM.render(React.createElement(Wrapper, {}), testRoot, function() {
+        t.deepEqual(events, [
+            ["mount"],
+            ["compute", 1],
+            ["computed label", 1],
+            ["render", 1, "1.1.hi.0", "hi"],
+        ])
+        events.splice(0)
+        $("#testDiv").click()
+
+        setTimeout(function() {
+            t.deepEqual(events, [
+                [ 'compute', 1 ],
+                [ 'react', 1 ],
+                [ 'receive', 1, 2 ],
+                [ 'update', 1, 2 ],
+                [ 'compute', 2 ],
+                [ "computed label", 2],
+                [ 'render', 2, '1.2.test.0', "test" ]
+            ])
+            t.end()
+        }, 100)
+    })
+})
+
+
+test('no re-render for shallow equal props', function(t) {
+    function createItem(subid, label) {
+        const res = mobx.observable({
+            id: 1,
+            label: label,
+        })
+        res.subid = subid // non reactive
+        return res
+    }
+
+    var data = mobx.observable({
+        items: [createItem(1, "hi")],
+        parentValue: 0
+    })
+    var events = []
+
+    var Child = observer(React.createClass({
+        componentWillMount() {
+            events.push(["mount"])
+        },
+
+        componentWillReceiveProps: function (nextProps) {
+            events.push(["receive", this.props.item.subid, nextProps.item.subid])
+        },
+
+        componentWillUpdate: function (nextProps) {
+            events.push(["update", this.props.item.subid, nextProps.item.subid])
+        },
+
+        componentWillReact: function() {
+            events.push(["react", this.props.item.subid])
+        },
+
+        render: function() {
+            events.push(["render", this.props.item.subid, this.props.item.label])
+            return React.createElement("span", {}, this.props.item.label)
+        }
+    }))
+
+    var Parent = observer(React.createClass({
+        render: function() {
+            t.equal(mobx.isObservable(this.props.nonObservable), false, "object has become observable!")
+            events.push(["parent render", data.parentValue])
+            return React.createElement("div", {
+                onClick: changeStuff.bind(this), // event is needed to get batching!
+                id: "testDiv"
+            }, data.items.map(function(item) {
+                return React.createElement(Child, {
+                    key: "fixed",
+                    item: item,
+                    value: 5
+                })
+            }))
+        }
+    }))
+
+    var Wrapper = React.createClass({ render: function() {
+        return React.createElement(Parent, { nonObservable: {} })
+    }})
+
+    function changeStuff() {
+        data.items[0].label = "hi" // no change
+        data.parentValue = 1 // rerender parent
+    }
+
+    ReactDOM.render(React.createElement(Wrapper, {}), testRoot, function() {
+        t.deepEqual(events, [
+            ["parent render", 0],
+            ["mount"],
+            ["render", 1, "hi"],
+        ])
+        events.splice(0)
+        $("#testDiv").click()
+
+        setTimeout(function() {
+            t.deepEqual(events, [
+                ["parent render", 1],
+                [ 'receive', 1, 1 ],
+            ])
+            t.end()
+        }, 100)
+    })
+})
+
+test('function passed in props is not invoked on property access', function(t) {
+  var Component = observer(React.createClass({
+    render: function() {
+      return React.createElement("div", {onClick: this.props.onClick})
+    }
+  }))
+  function onClick() {
+    t.fail(new Error("This function should not be called!"));
+  }
+  ReactDOM.render(React.createElement(Component, {onClick: onClick}), testRoot, function() {
+    t.end();
+  });
+})
+
+test('lifecycle callbacks called with correct arguments', function(t) {
+  t.timeoutAfter(200);
+  t.plan(6);
+  var Component = observer(React.createClass({
+    componentWillReceiveProps: function(nextProps) {
+      t.equal(nextProps.counter, 1, 'componentWillReceiveProps: nextProps.counter === 1');
+      t.equal(this.props.counter, 0, 'componentWillReceiveProps: this.props.counter === 1');
+    },
+    componentWillUpdate: function(nextProps, nextState) {
+      t.equal(nextProps.counter, 1, 'componentWillUpdate: nextProps.counter === 1');
+      t.equal(this.props.counter, 0, 'componentWillUpdate: this.props.counter === 0');
+    },
+    componentDidUpdate: function(prevProps, prevState) {
+      t.equal(prevProps.counter, 0, 'componentDidUpdate: prevProps.counter === 0');
+      t.equal(this.props.counter, 1, 'componentDidUpdate: this.props.counter === 1');
+    },
+    render: function() {
+      return React.createElement('div', {}, [
+        React.createElement('span', {key: "1"}, [this.props.counter]),
+        React.createElement('button', {key: "2", id: "testButton", onClick: this.props.onClick}),
+      ])
+    }
+  }))
+  var Root = React.createClass({
+    getInitialState: function() {
+      return {};
+    },
+    onButtonClick: function() {
+      this.setState({counter: (this.state.counter || 0) + 1})
+    },
+    render: function() {
+      return React.createElement(Component, {
+        counter: this.state.counter || 0,
+        onClick: this.onButtonClick,
+      })
+    },
+  })
+  ReactDOM.render(React.createElement(Root), testRoot, function() {
+    $("#testButton").click();
+  })
+})