[BUGFIX beta] allow watching of ES5+ Getter

packages/ember-metal/lib/properties.js

@@ -21,14 +21,33 @@ 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
 //
 
 export function MANDATORY_SETTER_FUNCTION(name) {
-  return function SETTER_FUNCTION(value) {
+  function SETTER_FUNCTION(value) {
     assert(`You must use Ember.set() to set the \`${name}\` property (of ${this}) to \`${value}\`.`, false);
-  };
+  }
+
+  SETTER_FUNCTION.isMandatorySetter = true;
+  return SETTER_FUNCTION;
 }
 
 export function DEFAULT_GETTER_FUNCTION(name) {
@@ -38,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
@@ -123,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;
         }
@@ -153,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/property_get.js

@@ -4,16 +4,12 @@
 
 import Ember from 'ember-metal/core';
 import { assert } from 'ember-metal/debug';
-import isEnabled from 'ember-metal/features';
 import EmberError from 'ember-metal/error';
 import {
   isGlobal as detectIsGlobal,
   isPath,
   hasThis as pathHasThis
 } from 'ember-metal/path_cache';
-import {
-  peekMeta
-} from 'ember-metal/meta';
 
 var FIRST_KEY = /^([^\.]+)/;
 
@@ -60,7 +56,6 @@ export function get(obj, keyName) {
     return obj;
   }
 
-  var meta = peekMeta(obj);
   var value = obj[keyName];
   var desc = (value !== null && typeof value === 'object' && value.isDescriptor) ? value : undefined;
   var ret;
@@ -72,15 +67,7 @@ export function get(obj, keyName) {
   if (desc) {
     return desc.get(obj, keyName);
   } else {
-    if (isEnabled('mandatory-setter')) {
-      if (meta && meta.peekWatching(keyName) > 0) {
-        ret = meta.peekValues(keyName);
-      } else {
-        ret = value;
-      }
-    } else {
-      ret = value;
-    }
+    ret = value;
 
     if (ret === undefined &&
         'object' === typeof obj && !(keyName in obj) && 'function' === typeof obj.unknownProperty) {

packages/ember-metal/lib/property_set.js

@@ -16,6 +16,10 @@ import {
   peekMeta
 } from 'ember-metal/meta';
 
+import {
+  lookupDescriptor
+} from 'ember-metal/utils';
+
 /**
   Sets the value of a property on an object, respecting computed properties
   and notifying observers and other listeners of the change. If the
@@ -69,23 +73,25 @@ export function set(obj, keyName, value, tolerant) {
       obj.setUnknownProperty(keyName, value);
     } else if (meta && meta.peekWatching(keyName) > 0) {
       if (meta.proto !== obj) {
-        if (isEnabled('mandatory-setter')) {
-          currentValue = meta.peekValues(keyName);
-        } else {
-          currentValue = obj[keyName];
-        }
+        currentValue = obj[keyName];
       }
       // only trigger a change if the value has changed
       if (value !== currentValue) {
         propertyWillChange(obj, keyName);
+
         if (isEnabled('mandatory-setter')) {
-          if (
-            (currentValue === undefined && !(keyName in obj)) ||
+          if ((currentValue === undefined && !(keyName in obj)) ||
             !Object.prototype.propertyIsEnumerable.call(obj, keyName)
           ) {
             defineProperty(obj, keyName, null, value); // setup mandatory setter
           } else {
-            meta.writeValues(keyName, value);
+            let descriptor = lookupDescriptor(obj, keyName);
+            let isMandatorySetter = descriptor && descriptor.set && descriptor.set.isMandatorySetter;
+            if (isMandatorySetter) {
+              meta.writeValues(keyName, value);
+            } else {
+              obj[keyName] = value;
+            }
           }
         } else {
           obj[keyName] = value;

packages/ember-metal/lib/utils.js

@@ -504,8 +504,25 @@ export function applyStr(t, m, a) {
   }
 }
 
+export 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;
+}
+
 export {
   GUID_KEY,
   makeArray,
   canInvoke
 };
+
+

packages/ember-metal/lib/watch_key.js

@@ -4,10 +4,12 @@ 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';
 
-let handleMandatorySetter, lookupDescriptor;
+let handleMandatorySetter;
 
 export function watchKey(obj, keyName, meta) {
   // can't watch length on Array - it is special...
@@ -20,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 {
@@ -37,48 +42,38 @@ export function watchKey(obj, keyName, meta) {
 
 
 if (isEnabled('mandatory-setter')) {
-  // It is true, the following code looks quite WAT. But have no fear, It
-  // exists purely to improve development ergonomics and is removed from
-  // ember.min.js and ember.prod.js builds.
-  //
-  // Some further context: Once a property is watched by ember, bypassing `set`
-  // for mutation, will bypass observation. This code exists to assert when
-  // that occurs, and attempt to provide more helpful feedback. The alternative
-  // is tricky to debug partially observable properties.
-  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;
-  };
-
+  // Future traveler, although this code looks scary. It merely exists in
+  // development to aid in development asertions. Production builds of
+  // ember strip this entire block out
   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;
     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);
     }
   };
 }
@@ -90,7 +85,10 @@ export function unwatchKey(obj, keyName, meta) {
     m.writeWatching(keyName, 0);
 
     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.didUnwatch) { desc.didUnwatch(obj, keyName); }
 
     if ('function' === typeof obj.didUnwatchProperty) {
@@ -107,21 +105,21 @@ export function unwatchKey(obj, keyName, meta) {
       // that occurs, and attempt to provide more helpful feedback. The alternative
       // is tricky to debug partially observable properties.
       if (!desc && keyName in obj) {
-        Object.defineProperty(obj, keyName, {
-          configurable: true,
-          enumerable: Object.prototype.propertyIsEnumerable.call(obj, keyName),
-          set(val) {
-            // redefine to set as enumerable
+        let maybeMandatoryDescriptor = lookupDescriptor(obj, keyName);
+
+        if (maybeMandatoryDescriptor.set && maybeMandatoryDescriptor.set.isMandatorySetter) {
+          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,
-              enumerable: true,
-              value: val
+              value: m.peekValues(keyName)
             });
             m.deleteFromValues(keyName);
-          },
-          get: DEFAULT_GETTER_FUNCTION(keyName)
-        });
+          }
+        }
       }
     }
   } else if (count > 1) {