Simplify ObserverSet implementation

Author: mmun

.eslintrc-browser.js

@@ -29,6 +29,7 @@ module.exports = {
     'clearInterval': true,
     'console': true,
 
+    'Map': true,
     'Set': true,
     'Symbol': true,
     'WeakMap': true,

packages/ember-metal/lib/index.js

@@ -52,7 +52,6 @@ export { default as isEmpty } from './is_empty';
 export { default as isBlank } from './is_blank';
 export { default as isPresent } from './is_present';
 export { default as run } from './run_loop';
-export { default as ObserverSet } from './observer_set';
 export {
   beginPropertyChanges,
   changeProperties,

packages/ember-metal/lib/observer_set.js

@@ -1,66 +1,55 @@
-import { guidFor } from 'ember-utils';
 import { sendEvent } from './events';
 
-/*
-  this.observerSet = {
-    [senderGuid]: { // variable name: `keySet`
-      [keyName]: listIndex
-    }
-  },
-  this.observers = [
-    {
-      sender: obj,
-      keyName: keyName,
-      eventName: eventName,
-      listeners: [
-        [target, method, flags]
-      ]
-    },
-    ...
-  ]
+/**
+  ObserverSet is a data structure used to keep track of observers
+  that have been deferred.
+
+  It ensures that observers are called in the same order that they
+  were initially triggered.
+
+  It also ensures that observers for any object-key pairs are called
+  only once, even if they were triggered multiple times while
+  deferred. In this case, the order that the observer is called in
+  will depend on the first time the observer was triggered.
+
+  @private
+  @class ObserverSet
 */
 export default class ObserverSet {
   constructor() {
-    this.clear();
+    this.added = new Map();
+    this.queue = [];
   }
 
-  add(sender, keyName, eventName) {
-    let observerSet = this.observerSet;
-    let observers = this.observers;
-    let senderGuid = guidFor(sender);
-    let keySet = observerSet[senderGuid];
-
-    if (keySet === undefined) {
-      observerSet[senderGuid] = keySet = {};
+  add(object, key) {
+    let keys = this.added.get(object);
+    if (keys === undefined) {
+      keys = new Set();
+      this.added.set(object, keys);
     }
 
-    let index = keySet[keyName];
-    if (index === undefined) {
-      index = observers.push({
-        sender,
-        keyName,
-        eventName,
-        listeners: []
-      }) - 1;
-      keySet[keyName] = index;
+    if (!keys.has(key)) {
+      this.queue.push({ object, key });
+      keys.add(key);
     }
-    return observers[index].listeners;
   }
 
   flush() {
-    let observers = this.observers;
-    let observer, sender;
-    this.clear();
-    for (let i = 0; i < observers.length; ++i) {
-      observer = observers[i];
-      sender = observer.sender;
-      if (sender.isDestroying || sender.isDestroyed) { continue; }
-      sendEvent(sender, observer.eventName, [sender, observer.keyName], observer.listeners);
+    for (let i = 0; i < this.queue.length; i++) {
+      let { object, key } = this.queue[i];
+
+      if (object.isDestroying || object.isDestroyed) {
+        continue;
+      }
+
+      sendEvent(object, key, [object, key]);
     }
+
+    this.clear();
   }
 
   clear() {
-    this.observerSet = {};
-    this.observers = [];
+    this.added.clear();
+    this.queue.length = 0;
   }
 }

packages/ember-metal/lib/property_events.js

@@ -220,46 +220,11 @@ function changeProperties(callback) {
   }
 }
 
-function indexOf(array, target, method) {
-  let index = -1;
-  // hashes are added to the end of the event array
-  // so it makes sense to start searching at the end
-  // of the array and search in reverse
-  for (let i = array.length - 3; i >= 0; i -= 3) {
-    if (target === array[i] && method === array[i + 1]) {
-      index = i;
-      break;
-    }
-  }
-  return index;
-}
-
-function accumulateListeners(obj, eventName, otherActions, meta) {
-  let actions = meta.matchingListeners(eventName);
-  if (actions === undefined) { return; }
-  let newActions = [];
-
-  for (let i = actions.length - 3; i >= 0; i -= 3) {
-    let target = actions[i];
-    let method = actions[i + 1];
-    let actionIndex = indexOf(otherActions, target, method);
-
-    if (actionIndex === -1) {
-      let flags = actions[i + 2];
-      otherActions.push(target, method, flags);
-      newActions.push(target, method, flags);
-    }
-  }
-
-  return newActions;
-}
-
 function notifyObservers(obj, keyName, meta) {
   if (meta.isSourceDestroying()) { return; }
 
   if (deferred > 0) {
-    let listeners = observerSet.add(obj, keyName, keyName);
-    accumulateListeners(obj, keyName, listeners, meta);
+    observerSet.add(obj, keyName);
   } else {
     sendEvent(obj, keyName, [obj, keyName]);
   }

packages/ember-metal/tests/observer_test.js

@@ -668,9 +668,9 @@ testBoth('observers added/removed during changeProperties should do the right th
   assert.equal(removedBeforeFirstChangeObserver.didChangeCount, 0, 'removeObserver called before the first change sees none');
   assert.equal(addedBeforeFirstChangeObserver.didChangeCount, 1, 'addObserver called before the first change sees only 1');
   assert.equal(addedAfterFirstChangeObserver.didChangeCount, 1, 'addObserver called after the first change sees 1');
-  assert.equal(addedAfterLastChangeObserver.didChangeCount, 0, 'addObserver called after the last change sees none');
-  assert.equal(removedBeforeLastChangeObserver.didChangeCount, 1, 'removeObserver called before the last change still sees 1');
-  assert.equal(removedAfterLastChangeObserver.didChangeCount, 1, 'removeObserver called after the last change still sees 1');
+  assert.equal(addedAfterLastChangeObserver.didChangeCount, 1, 'addObserver called after the last change sees 1');
+  assert.equal(removedBeforeLastChangeObserver.didChangeCount, 0, 'removeObserver called before the last change sees none');
+  assert.equal(removedAfterLastChangeObserver.didChangeCount, 0, 'removeObserver called after the last change sees none');
 });
 
 

packages/ember/lib/index.js

@@ -87,7 +87,6 @@ Ember.isEmpty = metal.isEmpty;
 Ember.isBlank = metal.isBlank;
 Ember.isPresent = metal.isPresent;
 Ember.run = metal.run;
-Ember._ObserverSet = metal.ObserverSet;
 Ember.propertyWillChange = metal.propertyWillChange;
 Ember.propertyDidChange = metal.propertyDidChange;
 Ember.notifyPropertyChange = metal.notifyPropertyChange;

packages/ember/tests/reexports_test.js

@@ -72,7 +72,6 @@ let allExports =[
   ['isPresent', 'ember-metal'],
   ['_Backburner', 'backburner', 'default'],
   ['run', 'ember-metal'],
-  ['_ObserverSet', 'ember-metal', 'ObserverSet'],
   ['propertyWillChange', 'ember-metal'],
   ['propertyDidChange', 'ember-metal'],
   ['notifyPropertyChange', 'ember-metal'],