Merge pull request #16919 from pzuraq/add-initializing-flag-to-meta

[refactor] Adds initialization flag and isPrototypeMeta()
emberjs · Oct 1, 2018 · 2390728 · 2390728
2 parents 7165960 + a80eba3
commit 2390728
Show file tree

Hide file tree

Showing 8 changed files with 129 additions and 35 deletions.
diff --git a/packages/@ember/-internals/meta/lib/meta.ts b/packages/@ember/-internals/meta/lib/meta.ts
@@ -33,9 +33,13 @@ if (DEBUG) {
 export const UNDEFINED = symbol('undefined');
 
 // FLAGS
-const SOURCE_DESTROYING = 1 << 1;
-const SOURCE_DESTROYED = 1 << 2;
-const META_DESTROYED = 1 << 3;
+const enum MetaFlags {
+  NONE = 0,
+  SOURCE_DESTROYING = 1 << 0,
+  SOURCE_DESTROYED = 1 << 1,
+  META_DESTROYED = 1 << 2,
+  INITIALIZING = 1 << 3,
+}
 
 export class Meta {
   _descriptors: any | undefined;
@@ -46,7 +50,7 @@ export class Meta {
   _chains: any | undefined;
   _tag: Tag | undefined;
   _tags: any | undefined;
-  _flags: number;
+  _flags: MetaFlags;
   source: object;
   proto: object | undefined;
   _parent: Meta | undefined | null;
@@ -74,13 +78,10 @@ export class Meta {
 
     // initial value for all flags right now is false
     // see FLAGS const for detailed list of flags used
-    this._flags = 0;
+    this._flags = MetaFlags.NONE;
 
     // used only internally
     this.source = obj;
-
-    // when meta(obj).proto === obj, the object is intended to be only a
-    // prototype and doesn't need to actually be observable itself
     this.proto = obj.constructor === undefined ? undefined : obj.constructor.prototype;
 
     this._listeners = undefined;
@@ -96,8 +97,20 @@ export class Meta {
     return parent;
   }
 
-  isInitialized(obj: object) {
-    return this.proto !== obj;
+  setInitializing() {
+    this._flags |= MetaFlags.INITIALIZING;
+  }
+
+  unsetInitializing() {
+    this._flags ^= MetaFlags.INITIALIZING;
+  }
+
+  isInitializing() {
+    return this._hasFlag(MetaFlags.INITIALIZING);
+  }
+
+  isPrototypeMeta(obj: object) {
+    return this.proto === this.source && this.source === obj;
   }
 
   destroy() {
@@ -114,27 +127,27 @@ export class Meta {
   }
 
   isSourceDestroying() {
-    return this._hasFlag(SOURCE_DESTROYING);
+    return this._hasFlag(MetaFlags.SOURCE_DESTROYING);
   }
 
   setSourceDestroying() {
-    this._flags |= SOURCE_DESTROYING;
+    this._flags |= MetaFlags.SOURCE_DESTROYING;
   }
 
   isSourceDestroyed() {
-    return this._hasFlag(SOURCE_DESTROYED);
+    return this._hasFlag(MetaFlags.SOURCE_DESTROYED);
   }
 
   setSourceDestroyed() {
-    this._flags |= SOURCE_DESTROYED;
+    this._flags |= MetaFlags.SOURCE_DESTROYED;
   }
 
   isMetaDestroyed() {
-    return this._hasFlag(META_DESTROYED);
+    return this._hasFlag(MetaFlags.META_DESTROYED);
   }
 
   setMetaDestroyed() {
-    this._flags |= META_DESTROYED;
+    this._flags |= MetaFlags.META_DESTROYED;
   }
 
   _hasFlag(flag: number) {
@@ -607,22 +620,36 @@ export function peekMeta(obj: object) {
     typeof obj === 'object' || typeof obj === 'function'
   );
 
-  let pointer = obj;
-  let meta;
+  if (DEBUG) {
+    counters!.peekCalls++;
+  }
+
+  let meta = metaStore.get(obj);
+
+  if (meta !== undefined) {
+    return meta;
+  }
+
+  let pointer = getPrototypeOf(obj);
+
   while (pointer !== undefined && pointer !== null) {
-    meta = metaStore.get(pointer);
-    // jshint loopfunc:true
     if (DEBUG) {
-      counters!.peekCalls++;
+      counters!.peekPrototypeWalks++;
     }
+
+    meta = metaStore.get(pointer);
+
     if (meta !== undefined) {
+      if (meta.proto !== pointer) {
+        // The meta was a prototype meta which was not marked as initializing.
+        // This can happen when a prototype chain was created manually via
+        // Object.create() and the source object does not have a constructor.
+        meta.proto = pointer;
+      }
       return meta;
     }
 
     pointer = getPrototypeOf(pointer);
-    if (DEBUG) {
-      counters!.peekPrototypeWalks++;
-    }
   }
 }
 

diff --git a/packages/@ember/-internals/metal/lib/properties.ts b/packages/@ember/-internals/metal/lib/properties.ts
@@ -61,7 +61,7 @@ interface ExtendedObject {
 export function MANDATORY_SETTER_FUNCTION(name: string): MandatorySetterFunction {
   function SETTER_FUNCTION(this: object, value: any | undefined | null) {
     let m = peekMeta(this);
-    if (!m.isInitialized(this)) {
+    if (m.isInitializing() || m.isPrototypeMeta(this)) {
       m.writeValues(name, value);
     } else {
       assert(

diff --git a/packages/@ember/-internals/metal/lib/property_events.ts b/packages/@ember/-internals/metal/lib/property_events.ts
@@ -90,7 +90,7 @@ function notifyPropertyChange(obj: object, keyName: string, _meta?: Meta): void
   let meta = _meta === undefined ? peekMeta(obj) : _meta;
   let hasMeta = meta !== undefined;
 
-  if (hasMeta && !meta.isInitialized(obj)) {
+  if (hasMeta && (meta.isInitializing() || meta.isPrototypeMeta(obj))) {
     return;
   }
 

diff --git a/packages/@ember/-internals/metal/tests/observer_test.js b/packages/@ember/-internals/metal/tests/observer_test.js
@@ -360,7 +360,7 @@ moduleFor(
 
       obj2.count = 0;
       set(obj, 'foo', 'baz');
-      assert.equal(obj.count, 1, 'should have invoked observer on parent');
+      assert.equal(obj.count, 0, 'should not have invoked observer on parent');
       assert.equal(obj2.count, 0, 'should not have invoked observer on inherited');
     }
 

diff --git a/packages/@ember/-internals/metal/tests/property_events_test.js b/packages/@ember/-internals/metal/tests/property_events_test.js
@@ -0,0 +1,67 @@
+import { meta } from '@ember/-internals/meta';
+import { notifyPropertyChange, PROPERTY_DID_CHANGE } from '..';
+import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
+
+moduleFor(
+  'notifyPropertyChange',
+  class extends AbstractTestCase {
+    ['@test notifies property changes on instances'](assert) {
+      class Foo {
+        [PROPERTY_DID_CHANGE](prop) {
+          assert.equal(prop, 'bar', 'property change notified');
+        }
+      }
+
+      let foo = new Foo();
+
+      notifyPropertyChange(foo, 'bar');
+    }
+
+    ['@test notifies property changes on instances with meta'](assert) {
+      class Foo {
+        [PROPERTY_DID_CHANGE](prop) {
+          assert.equal(prop, 'bar', 'property change notified');
+        }
+      }
+
+      let foo = new Foo();
+
+      meta(foo); // setup meta
+
+      notifyPropertyChange(foo, 'bar');
+    }
+
+    ['@test does not notify on class prototypes with meta'](assert) {
+      assert.expect(0);
+
+      class Foo {
+        [PROPERTY_DID_CHANGE](prop) {
+          assert.equal(prop, 'bar', 'property change notified');
+        }
+      }
+
+      let foo = new Foo();
+
+      meta(foo.constructor.prototype); // setup meta for prototype
+
+      notifyPropertyChange(foo.constructor.prototype, 'bar');
+    }
+
+    ['@test does not notify on non-class prototypes with meta'](assert) {
+      assert.expect(0);
+
+      let foo = {
+        [PROPERTY_DID_CHANGE](prop) {
+          assert.equal(prop, 'baz', 'property change notified');
+        },
+      };
+
+      let bar = Object.create(foo);
+
+      meta(foo); // setup meta for prototype
+      meta(bar); // setup meta for instance, switch prototype
+
+      notifyPropertyChange(foo, 'baz');
+    }
+  }
+);
diff --git a/packages/@ember/-internals/metal/tests/watching/watch_test.js b/packages/@ember/-internals/metal/tests/watching/watch_test.js
@@ -89,9 +89,9 @@ moduleFor(
       watch(obj, 'foo');
       assert.equal(get(obj, 'foo'), 'baz', 'should have original prop');
 
-      set(obj, 'foo', 'bar');
-      set(objB, 'foo', 'baz');
-      assert.equal(didCount, 2, 'should have invoked didCount once only');
+      set(objB, 'foo', 'bar');
+      set(obj, 'foo', 'baz');
+      assert.equal(didCount, 1, 'should have invoked didCount once only');
     }
 
     ['@test watching an object THEN defining it should work also'](assert) {

diff --git a/packages/@ember/-internals/runtime/lib/mixins/-proxy.js b/packages/@ember/-internals/runtime/lib/mixins/-proxy.js
@@ -90,9 +90,9 @@ export default Mixin.create({
   setUnknownProperty(key, value) {
     let m = meta(this);
 
-    if (m.proto === this) {
-      // if marked as prototype then just defineProperty
-      // rather than delegate
+    if (m.isInitializing() || m.isPrototypeMeta(this)) {
+      // if marked as prototype or object is initializing then just
+      // defineProperty rather than delegate
       defineProperty(this, key, null, value);
       return value;
     }

diff --git a/packages/@ember/-internals/runtime/lib/system/core_object.js b/packages/@ember/-internals/runtime/lib/system/core_object.js
@@ -129,7 +129,7 @@ function initialize(obj, properties) {
   obj.init(properties);
 
   // re-enable chains
-  m.proto = obj.constructor.prototype;
+  m.unsetInitializing();
   finishChains(m);
   sendEvent(obj, 'init', undefined, undefined, undefined, m);
 }
@@ -209,7 +209,7 @@ class CoreObject {
 
     // disable chains
     let m = meta(self);
-    m.proto = self;
+    m.setInitializing();
 
     if (properties !== DELAY_INIT) {
       deprecate(