another small optimization

mobxjs · Oct 20, 2018 · 4ef95bd · 4ef95bd
1 parent ab98fa2
commit 4ef95bd
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 29 deletions.
diff --git a/src/disposeOnUnmount.js b/src/disposeOnUnmount.js
@@ -46,7 +46,7 @@ export function disposeOnUnmount(target, propertyKeyOrFunction) {
 
     // tweak the component class componentWillUnmount if not done already
     if (!componentWasAlreadyModified) {
-        patch(target, "componentWillUnmount", false, runDisposersOnWillUnmount)
+        patch(target, "componentWillUnmount", runDisposersOnWillUnmount)
     }
 
     // return the disposer as is if invoked as a non decorator

diff --git a/src/observer.js b/src/observer.js
@@ -92,7 +92,7 @@ export const errorsReporter = new EventEmitter()
  */
 
 function patch(target, funcName) {
-    newPatch(target, funcName, false, reactiveMixin[funcName])
+    newPatch(target, funcName, reactiveMixin[funcName])
 }
 
 function shallowEqual(objA, objB) {

diff --git a/src/utils/utils.js b/src/utils/utils.js
@@ -54,7 +54,7 @@ function wrapFunction(mixins) {
     return fn
 }
 
-export function patch(target, methodName, forcePatch, ...mixinMethods) {
+export function patch(target, methodName, ...mixinMethods) {
     const mixins = getMixins(target, methodName)
 
     for (const mixinMethod of mixinMethods) {
@@ -64,18 +64,24 @@ export function patch(target, methodName, forcePatch, ...mixinMethods) {
     }
 
     const oldDefinition = Object.getOwnPropertyDescriptor(target, methodName)
-    if (!forcePatch && oldDefinition && oldDefinition[mobxPatchedDefinition]) {
+    if (oldDefinition && oldDefinition[mobxPatchedDefinition]) {
         // already patched definition, do not repatch
         return
     }
 
-    const newDefinition = createDefinition(target, methodName, oldDefinition, mixins)
+    const originalMethod = target[methodName]
+    const newDefinition = createDefinition(
+        target,
+        methodName,
+        oldDefinition ? oldDefinition.enumerable : undefined,
+        mixins,
+        originalMethod
+    )
 
     Object.defineProperty(target, methodName, newDefinition)
 }
 
-function createDefinition(target, methodName, oldDefinition, mixins) {
-    const originalMethod = target[methodName]
+function createDefinition(target, methodName, enumerable, mixins, originalMethod) {
     const wrappedFunc = wrapFunction(mixins)
     wrappedFunc[mobxRealMethod] = originalMethod
 
@@ -89,12 +95,14 @@ function createDefinition(target, methodName, oldDefinition, mixins) {
                 wrappedFunc[mobxRealMethod] = value
             } else {
                 // when it is an instance of the prototype/a child prototype patch that particular case again separately
-                // we don't need to pass any mixin functions since the structure is shared
-                patch(this, methodName, true)
-                this[methodName] = value
+                // since we need to store separate values depending on wether it is the actual instance, the prototype, etc
+                // e.g. the method for super might not be the same as the method for the prototype which might be not the same
+                // as the method for the instance
+                const newDefinition = createDefinition(this, methodName, enumerable, mixins, value)
+                Object.defineProperty(this, methodName, newDefinition)
             }
         },
         configurable: true,
-        enumerable: oldDefinition ? oldDefinition.enumerable : undefined
+        enumerable: enumerable
     }
 }
diff --git a/test/patch.test.js b/test/patch.test.js
@@ -33,8 +33,8 @@ test("no overrides", async () => {
             return null
         }
     }
-    patch(C.prototype, "componentDidMount", false, cdm)
-    patch(C.prototype, "componentWillUnmount", false, cwu)
+    patch(C.prototype, "componentDidMount", cdm)
+    patch(C.prototype, "componentWillUnmount", cwu)
 
     await testComponent(C, cdm, cwu)
 })
@@ -55,8 +55,8 @@ test("prototype overrides", async () => {
             return null
         }
     }
-    patch(C.prototype, "componentDidMount", false, cdm)
-    patch(C.prototype, "componentWillUnmount", false, cwu)
+    patch(C.prototype, "componentDidMount", cdm)
+    patch(C.prototype, "componentWillUnmount", cwu)
 
     await testComponent(C, cdm, cwu)
     expect(cdmCalls).toBe(1)
@@ -79,8 +79,8 @@ test("arrow function overrides", async () => {
             return null
         }
     }
-    patch(C.prototype, "componentDidMount", false, cdm)
-    patch(C.prototype, "componentWillUnmount", false, cwu)
+    patch(C.prototype, "componentDidMount", cdm)
+    patch(C.prototype, "componentWillUnmount", cwu)
 
     await testComponent(C, cdm, cwu)
     expect(cdmCalls).toBe(1)
@@ -109,8 +109,8 @@ test("recursive calls", async () => {
             return null
         }
     }
-    patch(C.prototype, "componentDidMount", false, cdm)
-    patch(C.prototype, "componentWillUnmount", false, cwu)
+    patch(C.prototype, "componentDidMount", cdm)
+    patch(C.prototype, "componentWillUnmount", cwu)
 
     await testComponent(C, cdm, cwu)
     expect(cdmCalls).toBe(10)
@@ -142,8 +142,8 @@ test("prototype + arrow function overrides", async () => {
             }
         }
     }
-    patch(C.prototype, "componentDidMount", false, cdm)
-    patch(C.prototype, "componentWillUnmount", false, cwu)
+    patch(C.prototype, "componentDidMount", cdm)
+    patch(C.prototype, "componentWillUnmount", cwu)
 
     await testComponent(C, cdm, cwu)
     expect(cdmCalls).toBe(1)
@@ -185,12 +185,12 @@ describe("inheritance with prototype methods", async () => {
         }
 
         if (patchBase) {
-            patch(B.prototype, "componentDidMount", false, cdm)
-            patch(B.prototype, "componentWillUnmount", false, cwu)
+            patch(B.prototype, "componentDidMount", cdm)
+            patch(B.prototype, "componentWillUnmount", cwu)
         }
         if (patchOther) {
-            patch(C.prototype, "componentDidMount", false, cdm)
-            patch(C.prototype, "componentWillUnmount", false, cwu)
+            patch(C.prototype, "componentDidMount", cdm)
+            patch(C.prototype, "componentWillUnmount", cwu)
         }
 
         await testComponent(C, cdm, cwu, patchBase || patchOther)
@@ -252,12 +252,12 @@ describe("inheritance with arrow functions", async () => {
         }
 
         if (patchBase) {
-            patch(B.prototype, "componentDidMount", false, cdm)
-            patch(B.prototype, "componentWillUnmount", false, cwu)
+            patch(B.prototype, "componentDidMount", cdm)
+            patch(B.prototype, "componentWillUnmount", cwu)
         }
         if (patchOther) {
-            patch(C.prototype, "componentDidMount", false, cdm)
-            patch(C.prototype, "componentWillUnmount", false, cwu)
+            patch(C.prototype, "componentDidMount", cdm)
+            patch(C.prototype, "componentWillUnmount", cwu)
         }
 
         await testComponent(C, cdm, cwu, patchBase || patchOther)