CHANGELOG.md

@@ -1,5 +1,9 @@
 # MobX-React Changelog
 
+### 5.3.4
+
+* Fixed unending recursing as a result of lifecylce patching. Fixes [#579](https://github.com/mobxjs/mobx-react/issues/579) through [#582](https://github.com/mobxjs/mobx-react/pull/582) by [@xaviergonz](https://github.com/xaviergonz)
+
 ### 5.3.3
 
 * Fixed `Cannot read property 'forEach' of undefined` exception if `disposeOnUnmount` was called conditionally. [#578](https://github.com/mobxjs/mobx-react/pull/578) by [Jef Hellemans](https://github.com/JefHellemans)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "mobx-react",
-  "version": "5.3.3",
+  "version": "5.3.4",
   "description": "React bindings for MobX. Create fully reactive components.",
   "main": "index.js",
   "jsnext:main": "index.module.js",

src/utils/utils.js

@@ -40,19 +40,33 @@ function createOrGetCachedDefinition(methodName, enumerable) {
 
     const wrapperMethod = function wrapperMethod(...args) {
         const mixins = getMixins(this, methodName)
+
+        // avoid possible recursive calls by custom patches
+        if (mixins.realRunning) {
+            return
+        }
+        mixins.realRunning = true
+
         const realMethod = mixins.real
+        let retVal
 
-        mixins.pre.forEach(pre => {
-            pre.apply(this, args)
-        })
+        try {
+            mixins.pre.forEach(pre => {
+                pre.apply(this, args)
+            })
 
-        if (realMethod !== undefined && realMethod !== null) {
-            realMethod.apply(this, args)
-        }
+            if (realMethod !== undefined && realMethod !== null) {
+                retVal = realMethod.apply(this, args)
+            }
+
+            mixins.post.forEach(post => {
+                post.apply(this, args)
+            })
 
-        mixins.post.forEach(post => {
-            post.apply(this, args)
-        })
+            return retVal
+        } finally {
+            mixins.realRunning = false
+        }
     }
     wrapperMethod[mobxMixin] = true
 

test/disposeOnUnmount.test.js

@@ -262,3 +262,161 @@ describe("with observer", () => {
         )
     })
 })
+
+test("custom patching should work", async () => {
+    class BaseComponent extends React.Component {
+        constructor(props, context) {
+            super(props, context)
+
+            _makeAllSafe(this, BaseComponent.prototype, [
+                "componentWillMount",
+                "componentDidMount",
+                "shouldComponentUpdate",
+                "componentWillUpdate",
+                "componentWillReceiveProps",
+                "render",
+                "componentDidUpdate",
+                "componentWillUnmount"
+            ])
+        }
+
+        componentDidMount() {
+            this.didMountCalled = true
+        }
+
+        componentWillUnmount() {
+            this.willUnmountCalled = true
+        }
+    }
+
+    function _makeAllSafe(obj, prototype, methodNames) {
+        for (let i = 0, len = methodNames.length; i < len; i++) {
+            _makeSafe(obj, prototype, methodNames[i])
+        }
+    }
+
+    function _makeSafe(obj, prototype, methodName) {
+        let classMethod = obj[methodName]
+        let prototypeMethod = prototype[methodName]
+
+        if (classMethod || prototypeMethod) {
+            obj[methodName] = function() {
+                this.patchRunFor = this.patchRunFor || []
+                this.patchRunFor.push(methodName)
+
+                let retVal
+
+                if (prototypeMethod) {
+                    retVal = prototypeMethod.apply(this, arguments)
+                }
+                if (classMethod !== prototypeMethod) {
+                    retVal = classMethod.apply(this, arguments)
+                }
+
+                return retVal
+            }
+        }
+    }
+
+    @observer
+    class C extends BaseComponent {
+        @disposeOnUnmount
+        methodA = jest.fn()
+        @disposeOnUnmount
+        methodB = jest.fn()
+        @disposeOnUnmount
+        methodC = null
+        @disposeOnUnmount
+        methodD = undefined
+
+        render() {
+            return null
+        }
+    }
+
+    await testComponent(
+        C,
+        ref => {
+            expect(ref.patchRunFor).toEqual(["render", "componentDidMount"])
+            expect(ref.didMountCalled).toBeTruthy()
+        },
+        ref => {
+            expect(ref.patchRunFor).toEqual(["render", "componentDidMount", "componentWillUnmount"])
+            expect(ref.willUnmountCalled).toBeTruthy()
+        }
+    )
+})
+
+it("componentDidMount should be different between components", async () => {
+    async function test(withObserver) {
+        const events = []
+
+        class A extends React.Component {
+            componentDidMount() {
+                this.didMount = "A"
+                events.push("mountA")
+            }
+
+            componentWillUnmount() {
+                this.willUnmount = "A"
+                events.push("unmountA")
+            }
+
+            render() {
+                return null
+            }
+        }
+
+        class B extends React.Component {
+            componentDidMount() {
+                this.didMount = "B"
+                events.push("mountB")
+            }
+
+            componentWillUnmount() {
+                this.willUnmount = "B"
+                events.push("unmountB")
+            }
+
+            render() {
+                return null
+            }
+        }
+
+        if (withObserver) {
+            A = observer(A)
+            B = observer(B)
+        }
+
+        const aRef = React.createRef()
+        await asyncReactDOMRender(<A ref={aRef} />, testRoot)
+        const caRef = aRef.current
+
+        expect(caRef.didMount).toBe("A")
+        expect(caRef.willUnmount).toBeUndefined()
+        expect(events).toEqual(["mountA"])
+
+        const bRef = React.createRef()
+        await asyncReactDOMRender(<B ref={bRef} />, testRoot)
+        const cbRef = bRef.current
+
+        expect(caRef.didMount).toBe("A")
+        expect(caRef.willUnmount).toBe("A")
+
+        expect(cbRef.didMount).toBe("B")
+        expect(cbRef.willUnmount).toBeUndefined()
+        expect(events).toEqual(["mountA", "unmountA", "mountB"])
+
+        await asyncReactDOMRender(null, testRoot)
+
+        expect(caRef.didMount).toBe("A")
+        expect(caRef.willUnmount).toBe("A")
+
+        expect(cbRef.didMount).toBe("B")
+        expect(cbRef.willUnmount).toBe("B")
+        expect(events).toEqual(["mountA", "unmountA", "mountB", "unmountB"])
+    }
+
+    await test(true)
+    await test(false)
+})