Merge pull request #15550 from bekzod/computed-sort-cleanup

avoid expanding already expanded property keys
emberjs · Jul 26, 2017 · 39161dc · 39161dc
2 parents be324a2 + 0b75175
commit 39161dc
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 34 deletions.
diff --git a/packages/ember-runtime/lib/computed/computed_macros.js b/packages/ember-runtime/lib/computed/computed_macros.js
@@ -37,20 +37,20 @@ function expandPropertiesToArray(predicateName, properties) {
 
 function generateComputedWithPredicate(name, predicate) {
   return (...properties) => {
-    let expandedProperties = expandPropertiesToArray(name, properties);
+    let dependentKeys = expandPropertiesToArray(name, properties);
 
     let computedFunc = new ComputedProperty(function() {
-      let lastIdx = expandedProperties.length - 1;
+      let lastIdx = dependentKeys.length - 1;
 
       for (let i = 0; i < lastIdx; i++) {
-        let value = get(this, expandedProperties[i]);
+        let value = get(this, dependentKeys[i]);
         if (!predicate(value)) {
           return value;
         }
       }
 
-      return get(this, expandedProperties[lastIdx]);
-    }, { dependentKeys: expandedProperties });
+      return get(this, dependentKeys[lastIdx]);
+    }, { dependentKeys });
 
     return computedFunc;
   };

diff --git a/packages/ember-runtime/lib/computed/reduce_computed_macros.js b/packages/ember-runtime/lib/computed/reduce_computed_macros.js
@@ -20,13 +20,13 @@ import { A as emberA } from '../system/native_array';
 
 
 function reduceMacro(dependentKey, callback, initialValue) {
-  return computed(`${dependentKey}.[]`, function() {
+  let cp = new ComputedProperty(function() {
     let arr = get(this, dependentKey);
-
     if (arr === null || typeof arr !== 'object') { return initialValue; }
-
     return arr.reduce(callback, initialValue, this);
-  }).readOnly();
+  }, { dependentKeys: [`${dependentKey}.[]`] })
+
+  return cp.readOnly();
 }
 
 function arrayMacro(dependentKey, callback) {
@@ -39,24 +39,26 @@ function arrayMacro(dependentKey, callback) {
     dependentKey += '.[]';
   }
 
-  return computed(dependentKey, function() {
+  let cp = new ComputedProperty(function() {
     let value = get(this, propertyName);
     if (isArray(value)) {
       return emberA(callback.call(this, value));
     } else {
       return emberA();
     }
-  }).readOnly();
+  }, { dependentKeys: [ dependentKey ] });
+
+  return cp.readOnly();
 }
 
-function multiArrayMacro(dependentKeys, callback) {
-  let args = dependentKeys.map(key => `${key}.[]`);
+function multiArrayMacro(_dependentKeys, callback) {
+  let dependentKeys = _dependentKeys.map(key => `${key}.[]`);
 
-  args.push(function() {
-    return emberA(callback.call(this, dependentKeys));
-  });
+  let cp = new ComputedProperty(function() {
+    return emberA(callback.call(this, _dependentKeys));
+  }, { dependentKeys });
 
-  return computed.apply(this, args).readOnly();
+  return cp.readOnly();
 }
 
 /**
@@ -716,50 +718,44 @@ function customSort(itemsKey, comparator) {
 // depending on the sortProperties
 function propertySort(itemsKey, sortPropertiesKey) {
   let cp = new ComputedProperty(function(key) {
-    let itemsKeyIsAtThis = (itemsKey === '@this');
     let sortProperties = get(this, sortPropertiesKey);
 
     assert(
       `The sort definition for '${key}' on ${this} must be a function or an array of strings`,
       isArray(sortProperties) && sortProperties.every(s => typeof s === 'string')
     );
 
-    let normalizedSortProperties = normalizeSortProperties(sortProperties);
-
     // Add/remove property observers as required.
     let activeObserversMap = cp._activeObserverMap || (cp._activeObserverMap = new WeakMap());
     let activeObservers = activeObserversMap.get(this);
 
-    if (activeObservers) {
+    if (activeObservers !== undefined) {
       activeObservers.forEach(args => removeObserver(...args));
     }
 
+    let itemsKeyIsAtThis = (itemsKey === '@this');
+    let items = itemsKeyIsAtThis ? this : get(this, itemsKey);
+    if (!isArray(items)) { return emberA(); }
+
     function sortPropertyDidChange() {
       this.notifyPropertyChange(key);
     }
 
+    let normalizedSortProperties = normalizeSortProperties(sortProperties);
     activeObservers = normalizedSortProperties.map(([prop]) => {
       let path = itemsKeyIsAtThis ? `@each.${prop}` : `${itemsKey}.@each.${prop}`;
-      let args = [this, path, sortPropertyDidChange];
-      addObserver(...args);
-      return args;
+      addObserver(this, path, sortPropertyDidChange);
+      return [this, path, sortPropertyDidChange];
     });
 
     activeObserversMap.set(this, activeObservers);
 
-    // Sort and return the array.
-    let items = itemsKeyIsAtThis ? this : get(this, itemsKey);
-
-    if (isArray(items)) {
-      return sortByNormalizedSortProperties(items, normalizedSortProperties);
-    } else {
-      return emberA();
-    }
-  });
+    return sortByNormalizedSortProperties(items, normalizedSortProperties);
+  }, { dependentKeys: [`${sortPropertiesKey}.[]`] });
 
   cp._activeObserverMap = undefined;
 
-  return cp.property(`${sortPropertiesKey}.[]`).readOnly();
+  return cp.readOnly();
 }
 
 function normalizeSortProperties(sortProperties) {