[BUGFIX beta] MandatorySetter should work with inheritance.

packages/ember-metal/lib/properties.js

@@ -21,6 +21,22 @@ export function Descriptor() {
   this.isDescriptor = true;
 }
 
+const REDEFINE_SUPPORTED = (function () {
+  // https://github.com/spalger/kibana/commit/b7e35e6737df585585332857a4c397dc206e7ff9
+  var a = Object.create(Object.prototype, {
+    prop: {
+      configurable: true,
+      value: 1
+    }
+  });
+
+  Object.defineProperty(a, 'prop', {
+    configurable: true,
+    value: 2
+  });
+
+  return a.prop === 2;
+}());
 // ..........................................................
 // DEFINING PROPERTIES API
 //
@@ -41,6 +57,16 @@ export function DEFAULT_GETTER_FUNCTION(name) {
   };
 }
 
+export function INHERITING_GETTER_FUNCTION(name) {
+  function IGETTER_FUNCTION() {
+    var proto = Object.getPrototypeOf(this);
+    return proto && proto[name];
+  }
+
+  IGETTER_FUNCTION.isInheritingGetter = true;
+  return IGETTER_FUNCTION;
+}
+
 /**
   NOTE: This is a low-level method used by other parts of the API. You almost
   never want to call this method directly. Instead you should use
@@ -126,12 +152,19 @@ export function defineProperty(obj, keyName, desc, data, meta) {
       if (isEnabled('mandatory-setter')) {
         if (watching) {
           meta.writeValues(keyName, data);
-          Object.defineProperty(obj, keyName, {
+
+          let defaultDescriptor = {
             configurable: true,
             enumerable: true,
             set: MANDATORY_SETTER_FUNCTION(keyName),
             get: DEFAULT_GETTER_FUNCTION(keyName)
-          });
+          };
+
+          if (REDEFINE_SUPPORTED) {
+            Object.defineProperty(obj, keyName, defaultDescriptor);
+          } else {
+            handleBrokenPhantomDefineProperty(obj, keyName, defaultDescriptor);
+          }
         } else {
           obj[keyName] = data;
         }
@@ -156,3 +189,9 @@ export function defineProperty(obj, keyName, desc, data, meta) {
 
   return this;
 }
+
+function handleBrokenPhantomDefineProperty(obj, keyName, desc) {
+  // https://github.com/ariya/phantomjs/issues/11856
+  Object.defineProperty(obj, keyName, { configurable: true, writable: true, value: 'iCry' });
+  Object.defineProperty(obj, keyName, desc);
+}

packages/ember-metal/lib/watch_key.js

@@ -4,7 +4,8 @@ import {
 } from 'ember-metal/meta';
 import {
   MANDATORY_SETTER_FUNCTION,
-  DEFAULT_GETTER_FUNCTION
+  DEFAULT_GETTER_FUNCTION,
+  INHERITING_GETTER_FUNCTION
 } from 'ember-metal/properties';
 import { lookupDescriptor } from 'ember-metal/utils';
 
@@ -21,14 +22,17 @@ export function watchKey(obj, keyName, meta) {
     m.writeWatching(keyName, 1);
 
     var possibleDesc = obj[keyName];
-    var desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined;
+    var desc = (possibleDesc !== null &&
+                typeof possibleDesc === 'object' &&
+                possibleDesc.isDescriptor) ? possibleDesc : undefined;
     if (desc && desc.willWatch) { desc.willWatch(obj, keyName); }
 
     if ('function' === typeof obj.willWatchProperty) {
       obj.willWatchProperty(keyName);
     }
 
     if (isEnabled('mandatory-setter')) {
+      // NOTE: this is dropped for prod + minified builds
       handleMandatorySetter(m, obj, keyName);
     }
   } else {
@@ -47,19 +51,29 @@ if (isEnabled('mandatory-setter')) {
     var isWritable = descriptor ? descriptor.writable : true;
     var hasValue = descriptor ? 'value' in descriptor : true;
     var possibleDesc = descriptor && descriptor.value;
-    var isDescriptor = possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor;
+    var isDescriptor = possibleDesc !== null &&
+                       typeof possibleDesc === 'object' &&
+                       possibleDesc.isDescriptor;
 
     if (isDescriptor) { return; }
 
     // this x in Y deopts, so keeping it in this function is better;
     if (configurable && isWritable && hasValue && keyName in obj) {
-      m.writeValues(keyName, obj[keyName]);
-      Object.defineProperty(obj, keyName, {
+      let desc = {
         configurable: true,
         enumerable: Object.prototype.propertyIsEnumerable.call(obj, keyName),
         set: MANDATORY_SETTER_FUNCTION(keyName),
-        get: DEFAULT_GETTER_FUNCTION(keyName)
-      });
+        get: undefined
+      };
+
+      if (Object.prototype.hasOwnProperty.call(obj, keyName)) {
+        m.writeValues(keyName, obj[keyName]);
+        desc.get = DEFAULT_GETTER_FUNCTION(keyName);
+      } else {
+        desc.get = INHERITING_GETTER_FUNCTION(keyName);
+      }
+
+      Object.defineProperty(obj, keyName, desc);
     }
   };
 }
@@ -94,14 +108,17 @@ export function unwatchKey(obj, keyName, meta) {
         let maybeMandatoryDescriptor = lookupDescriptor(obj, keyName);
 
         if (maybeMandatoryDescriptor.set && maybeMandatoryDescriptor.set.isMandatorySetter) {
-          let isEnumerable = Object.prototype.propertyIsEnumerable.call(obj, keyName);
-          Object.defineProperty(obj, keyName, {
-            configurable: true,
-            enumerable: isEnumerable,
-            writable: true,
-            value: m.peekValues(keyName)
-          });
-          m.deleteFromValues(keyName);
+          if (maybeMandatoryDescriptor.get && maybeMandatoryDescriptor.get.isInheritingGetter) {
+            delete obj[keyName];
+          } else {
+            Object.defineProperty(obj, keyName, {
+              configurable: true,
+              enumerable: Object.prototype.propertyIsEnumerable.call(obj, keyName),
+              writable: true,
+              value: m.peekValues(keyName)
+            });
+            m.deleteFromValues(keyName);
+          }
         }
       }
     }

packages/ember-metal/tests/accessors/mandatory_setters_test.js

@@ -355,6 +355,7 @@ if (isEnabled('mandatory-setter')) {
         return 'custom-object';
       }
     };
+
     var obj2 = Object.create(obj);
 
     watch(obj2, 'someProp');
@@ -365,4 +366,69 @@ if (isEnabled('mandatory-setter')) {
       obj2.someProp = 'foo-bar';
     }, 'You must use Ember.set() to set the `someProp` property (of custom-object) to `foo-bar`.');
   });
+
+  QUnit.test('inheritance remains live', function() {
+    function Parent() {}
+    Parent.prototype.food  = 'chips';
+
+    var child = new Parent();
+
+    equal(child.food , 'chips');
+
+    watch(child, 'food');
+
+    equal(child.food , 'chips');
+
+    Parent.prototype.food  = 'icecreame';
+
+    equal(child.food , 'icecreame');
+
+    unwatch(child, 'food');
+
+    equal(child.food, 'icecreame');
+
+    Parent.prototype.food  = 'chips';
+
+    equal(child.food, 'chips');
+  });
+
+
+  QUnit.test('inheritance remains live and preserves this', function() {
+    function Parent(food) {
+      this._food = food;
+    }
+
+    Object.defineProperty(Parent.prototype, 'food', {
+      get() {
+        return this._food;
+      }
+    });
+
+    let child = new Parent('chips');
+
+    equal(child.food , 'chips');
+
+    watch(child, 'food');
+
+    equal(child.food , 'chips');
+
+    child._food  = 'icecreame';
+
+    equal(child.food , 'icecreame');
+
+    unwatch(child, 'food');
+
+    equal(child.food, 'icecreame');
+
+    let foodDesc = Object.getOwnPropertyDescriptor(Parent.prototype, 'food');
+    ok(!foodDesc.configurable, 'Parent.prototype.food desc should be non configable');
+    ok(!foodDesc.enumerable, 'Parent.prototype.food desc should be non enumerable');
+
+    equal(foodDesc.get.call({
+      _food: 'hi'
+    }), 'hi');
+    equal(foodDesc.set, undefined);
+
+    equal(child.food, 'icecreame');
+  });
 }

packages/ember-metal/tests/keys_test.js

@@ -162,6 +162,7 @@ QUnit.test('observer switched on and off and then setter', function () {
   addObserver(beer, 'type', K);
   removeObserver(beer, 'type', K);
 
+  deepEqual(Object.keys(beer), [], 'addObserver -> removeObserver');
   set(beer, 'type', 'ale');
 
   deepEqual(Object.keys(beer), ['type'], 'addObserver -> removeObserver -> set');