Merge pull request #12314 from rwjblue/check-prototype-for-setters

[BUGFIX beta] Mandatory setter should check prototype descriptors.
emberjs · Oct 13, 2015 · ecbc94a · ecbc94a
2 parents 2816388 + 9d3a62d
commit ecbc94a
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 2 deletions.
diff --git a/packages/ember-metal/lib/watch_key.js b/packages/ember-metal/lib/watch_key.js
@@ -7,6 +7,8 @@ import {
   DEFAULT_GETTER_FUNCTION
 } from 'ember-metal/properties';
 
+let handleMandatorySetter, lookupDescriptor;
+
 export function watchKey(obj, keyName, meta) {
   // can't watch length on Array - it is special...
   if (keyName === 'length' && Array.isArray(obj)) { return; }
@@ -35,8 +37,23 @@ export function watchKey(obj, keyName, meta) {
 
 
 if (isEnabled('mandatory-setter')) {
-  var handleMandatorySetter = function handleMandatorySetter(m, obj, keyName) {
-    var descriptor = Object.getOwnPropertyDescriptor && Object.getOwnPropertyDescriptor(obj, keyName);
+  lookupDescriptor = function lookupDescriptor(obj, keyName) {
+    let current = obj;
+    while (current) {
+      let descriptor = Object.getOwnPropertyDescriptor(current, keyName);
+
+      if (descriptor) {
+        return descriptor;
+      }
+
+      current = Object.getPrototypeOf(current);
+    }
+
+    return null;
+  };
+
+  handleMandatorySetter = function handleMandatorySetter(m, obj, keyName) {
+    let descriptor = lookupDescriptor(obj, keyName);
     var configurable = descriptor ? descriptor.configurable : true;
     var isWritable = descriptor ? descriptor.writable : true;
     var hasValue = descriptor ? 'value' in descriptor : true;

diff --git a/packages/ember-metal/tests/accessors/mandatory_setters_test.js b/packages/ember-metal/tests/accessors/mandatory_setters_test.js
@@ -56,11 +56,96 @@ if (isEnabled('mandatory-setter')) {
     var obj = { someProp: null };
 
     Object.defineProperty(obj, 'someProp', {
+      get() {
+        return null;
+      },
+
+      set(value) {
+        equal(value, 'foo-bar', 'custom setter was called');
+      }
+    });
+
+    watch(obj, 'someProp');
+    ok(!hasMandatorySetter(obj, 'someProp'), 'mandatory-setter should not be installed');
+
+    obj.someProp = 'foo-bar';
+  });
+
+  QUnit.test('should not setup mandatory-setter if setter is already setup on property in parent prototype', function() {
+    expect(2);
+
+    function Foo() { }
+
+    Object.defineProperty(Foo.prototype, 'someProp', {
+      get() {
+        return null;
+      },
+
+      set(value) {
+        equal(value, 'foo-bar', 'custom setter was called');
+      }
+    });
+
+    var obj = new Foo();
+
+    watch(obj, 'someProp');
+    ok(!hasMandatorySetter(obj, 'someProp'), 'mandatory-setter should not be installed');
+
+    obj.someProp = 'foo-bar';
+  });
+
+  QUnit.test('should not setup mandatory-setter if setter is already setup on property in grandparent prototype', function() {
+    expect(2);
+
+    function Foo() { }
+
+    Object.defineProperty(Foo.prototype, 'someProp', {
+      get() {
+        return null;
+      },
+
+      set(value) {
+        equal(value, 'foo-bar', 'custom setter was called');
+      }
+    });
+
+    function Bar() { }
+    Bar.prototype = Object.create(Foo.prototype);
+    Bar.prototype.constructor = Bar;
+
+    var obj = new Bar();
+
+    watch(obj, 'someProp');
+    ok(!hasMandatorySetter(obj, 'someProp'), 'mandatory-setter should not be installed');
+
+    obj.someProp = 'foo-bar';
+  });
+
+  QUnit.test('should not setup mandatory-setter if setter is already setup on property in great grandparent prototype', function() {
+    expect(2);
+
+    function Foo() { }
+
+    Object.defineProperty(Foo.prototype, 'someProp', {
+      get() {
+        return null;
+      },
+
       set(value) {
         equal(value, 'foo-bar', 'custom setter was called');
       }
     });
 
+    function Bar() { }
+    Bar.prototype = Object.create(Foo.prototype);
+    Bar.prototype.constructor = Bar;
+
+    function Qux() { }
+    Qux.prototype = Object.create(Bar.prototype);
+    Qux.prototype.constructor = Qux;
+
+    var obj = new Qux();
+
     watch(obj, 'someProp');
     ok(!hasMandatorySetter(obj, 'someProp'), 'mandatory-setter should not be installed');