Fixed stack overflow for @computed on RN, fixes #1777

mobxjs · Dec 3, 2018 · 8351262 · 8351262
1 parent 16f8586
commit 8351262
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 20 deletions.
diff --git a/src/api/computed.ts b/src/api/computed.ts
@@ -28,8 +28,9 @@ export const computedDecorator = createPropDecorator(
         const { get, set } = descriptor // initialValue is the descriptor for get / set props
         // Optimization: faster on decorator target or instance? Assuming target
         // Optimization: find out if declaring on instance isn't just faster. (also makes the property descriptor simpler). But, more memory usage..
+        // Forcing instance now, fixes hot reloadig issues on React Native:
         const options = decoratorArgs[0] || {}
-        asObservableObject(instance).addComputedProp(decoratorTarget, propertyName, {
+        asObservableObject(instance).addComputedProp(instance, propertyName, {
             get,
             set,
             context: instance,

diff --git a/test/base/babel-tests.js b/test/base/babel-tests.js
@@ -574,7 +574,7 @@ test("enumerability", () => {
         @computed
         get b() {
             return this.a
-        } // non-enumerable, on proto
+        } // non-enumerable, (and, ideally, on proto)
         @action
         m() {} // non-enumerable, on proto
         @action m2 = () => {} // non-enumerable, on self
@@ -587,17 +587,15 @@ test("enumerability", () => {
     let props = []
     for (var key in a) props.push(key)
 
-    expect(ownProps).toEqual(
-        [
-            // should have a, not supported yet in babel...
-        ]
-    )
+    expect(ownProps).toEqual([
+        // should have a, not supported yet in babel...
+    ])
 
     expect(props).toEqual(["a", "a2"])
 
     expect("a" in a).toBe(true)
-    expect(a.hasOwnProperty("a")).toBe(false) // true would better..
-    expect(a.hasOwnProperty("b")).toBe(false)
+    expect(a.hasOwnProperty("a")).toBe(false)
+    expect(a.hasOwnProperty("b")).toBe(false) // true would be more consistent, see below
     expect(a.hasOwnProperty("m")).toBe(false)
     expect(a.hasOwnProperty("m2")).toBe(true)
 
@@ -624,7 +622,7 @@ test("enumerability", () => {
     expect("a" in a).toBe(true)
     expect(a.hasOwnProperty("a")).toBe(true)
     expect(a.hasOwnProperty("a2")).toBe(true)
-    expect(a.hasOwnProperty("b")).toBe(false) // true would also be ok-ish. see: #1398
+    expect(a.hasOwnProperty("b")).toBe(true) // true would better.. but, #1777
     expect(a.hasOwnProperty("m")).toBe(false)
     expect(a.hasOwnProperty("m2")).toBe(true)
 })
@@ -636,7 +634,7 @@ test("enumerability - workaround", () => {
         @computed
         get b() {
             return this.a
-        } // non-enumerable, on proto
+        } // non-enumerable, (and, ideally, on proto)
         @action
         m() {} // non-enumerable, on proto
         @action m2 = () => {} // non-enumerable, on self
@@ -663,7 +661,7 @@ test("enumerability - workaround", () => {
     expect("a" in a).toBe(true)
     expect(a.hasOwnProperty("a")).toBe(true)
     expect(a.hasOwnProperty("a2")).toBe(true)
-    expect(a.hasOwnProperty("b")).toBe(false) // true would also be ok-ish. see: #1398
+    expect(a.hasOwnProperty("b")).toBe(true) // ideally, false, but #1777
     expect(a.hasOwnProperty("m")).toBe(false)
     expect(a.hasOwnProperty("m2")).toBe(true)
 })
@@ -701,11 +699,9 @@ test("verify object assign (babel)", () => {
     }
 
     const todo = new Todo()
-    expect(Object.assign({}, todo)).toEqual(
-        {
-            //		Should be:	title: "test"!
-        }
-    )
+    expect(Object.assign({}, todo)).toEqual({
+        //		Should be:	title: "test"!
+    })
 
     todo.title // lazy initialization :'(
 

diff --git a/test/base/typescript-tests.ts b/test/base/typescript-tests.ts
@@ -802,7 +802,7 @@ test("enumerability", () => {
         @computed
         get b() {
             return this.a
-        } // non-enumerable, on proto
+        } // non-enumerable, (and, ideally, on proto)
         @action
         m() {} // non-enumerable, on proto
         @action m2 = () => {} // non-enumerable, on self
@@ -826,7 +826,7 @@ test("enumerability", () => {
 
     t.equal("a" in a, true)
     t.equal(a.hasOwnProperty("a"), true)
-    t.equal(a.hasOwnProperty("b"), false) // ok also ok-ish
+    t.equal(a.hasOwnProperty("b"), true) // false would be slightly better, true also ok-ish, and, see #1777
     t.equal(a.hasOwnProperty("m"), false)
     t.equal(a.hasOwnProperty("m2"), true)
 
@@ -849,7 +849,7 @@ test("enumerability", () => {
 
     t.equal("a" in a, true)
     t.equal(a.hasOwnProperty("a"), true)
-    t.equal(a.hasOwnProperty("b"), false) // true would also be ok-ish, see: #1398
+    t.equal(a.hasOwnProperty("b"), true) // false would be slightly better, true also ok-ish, and, see #1777
     t.equal(a.hasOwnProperty("m"), false)
     t.equal(a.hasOwnProperty("m2"), true)
 })