diff --git a/src/observer.js b/src/observer.js index 7e192bac..29e90f46 100644 --- a/src/observer.js +++ b/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) diff --git a/test/issue21.test.js b/test/issue21.test.js index d1ace35a..81125b73 100644 --- a/test/issue21.test.js +++ b/test/issue21.test.js @@ -449,3 +449,55 @@ test("lifecycle callbacks called with correct arguments", async () => { await asyncReactDOMRender(, 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
+ } + } + ) + + await asyncReactDOMRender(, testRoot) + await asyncReactDOMRender(, testRoot) + await asyncReactDOMRender(, testRoot) + await asyncReactDOMRender(, testRoot) + expect(constructorCallsCount).toEqual(1) + expect(componentWillMountCallsCount).toEqual(1) + expect(prop1Values).toEqual(["1", "2", "3", "4"]) + expect(prop2Values).toEqual(["4", "3", "2", "1"]) +})