Merge pull request emberjs#4947 from emberjs/remove-change-removes

remove needless change events when creating a recordArrays
rwjblue · May 16, 2017 · 58bd396 · 58bd396
2 parents 4914cbd + a36a44f
commit 58bd396
Show file tree

Hide file tree

Showing 10 changed files with 371 additions and 144 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -2,22 +2,30 @@
 language: node_js
 sudo: false
 node_js:
-  - "6.9"
+  - "7"
+
+cache:
+  yarn: true
+
 before_install:
-  - "npm config set spin false"
-  - "npm install -g npm@^2"
+  - "export PATH=$PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64/bin:$PATH"
+  - "if [ $(phantomjs --version) != '2.1.1' ]; then rm -rf $PWD/travis_phantomjs; mkdir -p $PWD/travis_phantomjs; fi"
+  - "if [ $(phantomjs --version) != '2.1.1' ]; then wget https://assets.membergetmember.co/software/phantomjs-2.1.1-linux-x86_64.tar.bz2 -O $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2; fi"
+  - "if [ $(phantomjs --version) != '2.1.1' ]; then tar -xvf $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 -C $PWD/travis_phantomjs; fi"
+  - "phantomjs --version"
 
 install:
-  - npm install --no-optional
+  - yarn
   - ./node_modules/.bin/bower install
+
 script:
   - ./bin/lint-features
-  - npm run-script test
-  - npm run-script test:optional-features
-  - npm run-script test:production
-  - npm run-script node-tests
+  - yarn run test
+  - yarn run test:optional-features
+  - yarn run test:production
+  - yarn run node-tests
 after_success:
-  - npm run-script production
+  - yarn run production
   - "./bin/publish-builds"
 env:
   global:

diff --git a/addon/-private/system/record-array-manager.js b/addon/-private/system/record-array-manager.js
@@ -8,10 +8,13 @@ import {
   FilteredRecordArray,
   AdapterPopulatedRecordArray
 } from "./record-arrays";
+
+import cloneNull from "./clone-null";
 import { assert } from '@ember/debug';
 
 const {
   get,
+  set,
   run: emberRun
 } = Ember;
 
@@ -25,7 +28,6 @@ const {
   createRecordArray,
   liveRecordArrayFor,
   filteredRecordArraysFor,
-  populateLiveRecordArray,
   recordDidChange,
   registerFilteredRecordArray,
   unregisterRecordArray,
@@ -41,7 +43,6 @@ const {
   'createRecordArray',
   'liveRecordArrayFor',
   'filteredRecordArraysFor',
-  'populateLiveRecordArray',
   'recordDidChange',
   'registerFilteredRecordArray',
   'unregisterRecordArray',
@@ -126,59 +127,22 @@ export default class RecordArrayManager {
         }
       }
 
-      // TODO: skip if it only changed
-      // process liveRecordArrays
-      if (this._liveRecordArrays[modelName]) {
-        this.updateLiveRecordArray(modelName, internalModels);
+      let array = this._liveRecordArrays[modelName];
+      if (array) {
+        // TODO: skip if it only changed
+        // process liveRecordArrays
+        this.updateLiveRecordArray(array, internalModels);
       }
 
       // process adapterPopulatedRecordArrays
       if (modelsToRemove.length > 0) {
-        this.removeFromAdapterPopulatedRecordArrays(modelsToRemove);
+        removeFromAdapterPopulatedRecordArrays(modelsToRemove);
       }
     }
   }
 
-  updateLiveRecordArray(modelName, internalModels) {
-    let array = this.liveRecordArrayFor(modelName);
-
-    let modelsToAdd = [];
-    let modelsToRemove = [];
-
-    for (let i = 0; i < internalModels.length; i++) {
-      let internalModel = internalModels[i];
-      let isDeleted = internalModel.isHiddenFromRecordArrays();
-      let recordArrays = internalModel._recordArrays;
-
-      if (!isDeleted && !internalModel.isEmpty()) {
-        if (!recordArrays.has(array)) {
-          modelsToAdd.push(internalModel);
-          recordArrays.add(array);
-        }
-      }
-
-      if (isDeleted) {
-        modelsToRemove.push(internalModel);
-        recordArrays.delete(array)
-      }
-    }
-
-    if (modelsToAdd.length > 0)    { array._pushInternalModels(modelsToAdd); }
-    if (modelsToRemove.length > 0) { array._removeInternalModels(modelsToRemove); }
-  }
-
-  removeFromAdapterPopulatedRecordArrays(internalModels) {
-    for (let i = 0; i < internalModels.length; i++) {
-      let internalModel = internalModels[i];
-      let list = internalModel._recordArrays.list;
-
-      for (let j = 0; j < list.length; j++) {
-        // TODO: group by arrays, so we can batch remove
-        list[j]._removeInternalModels([internalModel]);
-      }
-
-      internalModel._recordArrays.clear();
-    }
+  updateLiveRecordArray(array, internalModels) {
+    return updateLiveRecordArray(array, internalModels);
   }
 
   /**
@@ -217,7 +181,7 @@ export default class RecordArrayManager {
   }
 
   // TODO: remove, utilize existing flush code but make it flush sync based on 1 modelName
-  syncLiveRecordArray(array, modelName) {
+  _syncLiveRecordArray(array, modelName) {
     assert(`recordArrayManger.syncLiveRecordArray expects modelName not modelClass as the second param`, typeof modelName === 'string');
     let hasNoPotentialDeletions = Object.keys(this._pending).length === 0;
     let map = this.store._internalModelsFor(modelName);
@@ -228,29 +192,19 @@ export default class RecordArrayManager {
       liveRecordArrays, and is capable of strategically flushing those changes and applying
       small diffs if desired.  However, until we've refactored recordArrayManager, this dirty
       check prevents us from unnecessarily wiping out live record arrays returned by peekAll.
-     */
+      */
     if (hasNoPotentialDeletions && hasNoInsertionsOrRemovals) {
       return;
     }
 
-    this.populateLiveRecordArray(array, map.models);
-  }
-
-  // TODO: remove, when syncLiveRecordArray is removed
-  populateLiveRecordArray(array, internalModels) {
-    heimdall.increment(populateLiveRecordArray);
-
+    let internalModels = this._visibleInternalModelsByType(modelName);
     let modelsToAdd = [];
     for (let i = 0; i < internalModels.length; i++) {
       let internalModel = internalModels[i];
-
-      if (!internalModel.isHiddenFromRecordArrays()) {
-        let recordArrays = internalModel._recordArrays;
-
-        if (!recordArrays.has(array)) {
-          modelsToAdd.push(internalModel);
-          recordArrays.add(array);
-        }
+      let recordArrays = internalModel._recordArrays;
+      if (recordArrays.has(array) === false) {
+        recordArrays.add(array);
+        modelsToAdd.push(internalModel);
       }
     }
 
@@ -278,6 +232,13 @@ export default class RecordArrayManager {
     this.updateFilterRecordArray(array, filter, internalModels);
   }
 
+  _didUpdateAll(modelName) {
+    let recordArray = this._liveRecordArrays[modelName];
+    if (recordArray) {
+      set(recordArray, 'isUpdating', false);
+    }
+  }
+
   /**
     Get the `DS.RecordArray` for a modelName, which contains all loaded records of
     given modelName.
@@ -291,9 +252,33 @@ export default class RecordArrayManager {
 
     heimdall.increment(liveRecordArrayFor);
 
-    return this._liveRecordArrays[modelName] || (this._liveRecordArrays[modelName] = this.createRecordArray(modelName))
+    let array = this._liveRecordArrays[modelName];
+
+    if (array) {
+      // if the array already exists, synchronize
+      this._syncLiveRecordArray(array, modelName);
+    } else {
+      // if the array is being newly created merely create it with its initial
+      // content already set. This prevents unneeded change events.
+      let internalModels = this._visibleInternalModelsByType(modelName);
+      array = this.createRecordArray(modelName, internalModels);
+      this._liveRecordArrays[modelName] = array;
+    }
+
+    return array;
   }
 
+  _visibleInternalModelsByType(modelName) {
+    let all = this.store._internalModelsFor(modelName)._models;
+    let visible = [];
+    for (let i = 0; i < all.length; i++) {
+      let model = all[i];
+      if (model.isHiddenFromRecordArrays() === false) {
+        visible.push(model);
+      }
+    }
+    return visible;
+  }
   /**
     Get the `DS.RecordArray` for a modelName, which contains all loaded records of
     given modelName.
@@ -314,18 +299,26 @@ export default class RecordArrayManager {
 
     @method createRecordArray
     @param {String} modelName
+    @param {Array} _content (optional|private)
     @return {DS.RecordArray}
   */
-  createRecordArray(modelName) {
+  createRecordArray(modelName, content) {
     assert(`recordArrayManger.createRecordArray expects modelName not modelClass as the param`, typeof modelName === 'string');
     heimdall.increment(createRecordArray);
-    return RecordArray.create({
+
+    let array = RecordArray.create({
       modelName,
-      content: Ember.A(),
+      content: Ember.A(content || []),
       store: this.store,
       isLoaded: true,
       manager: this
     });
+
+    if (Array.isArray(content)) {
+      associateWithRecordArray(content, array);
+    }
+
+    return array;
   }
 
   /**
@@ -363,17 +356,34 @@ export default class RecordArrayManager {
     @param {Object} query
     @return {DS.AdapterPopulatedRecordArray}
   */
-  createAdapterPopulatedRecordArray(modelName, query) {
+  createAdapterPopulatedRecordArray(modelName, query, internalModels, payload) {
     heimdall.increment(createAdapterPopulatedRecordArray);
     assert(`recordArrayManger.createAdapterPopulatedRecordArray expects modelName not modelClass as the first param, received ${modelName}`, typeof modelName === 'string');
 
-    let array = AdapterPopulatedRecordArray.create({
-      modelName,
-      query: query,
-      content: Ember.A(),
-      store: this.store,
-      manager: this
-    });
+    let array;
+    if (Array.isArray(internalModels)) {
+      array = AdapterPopulatedRecordArray.create({
+        modelName,
+        query: query,
+        content: Ember.A(internalModels),
+        store: this.store,
+        manager: this,
+        isLoaded: true,
+        isUpdating: false,
+        meta: cloneNull(payload.meta),
+        links: cloneNull(payload.links)
+      });
+
+      associateWithRecordArray(internalModels, array);
+    } else {
+      array = AdapterPopulatedRecordArray.create({
+        modelName,
+        query: query,
+        content: Ember.A(),
+        store: this.store,
+        manager: this
+      });
+    }
 
     this._adapterPopulatedRecordArrays.push(array);
 
@@ -470,3 +480,50 @@ function remove(array, item) {
 
   return false;
 }
+
+function updateLiveRecordArray(array, internalModels) {
+  let modelsToAdd = [];
+  let modelsToRemove = [];
+
+  for (let i = 0; i < internalModels.length; i++) {
+    let internalModel = internalModels[i];
+    let isDeleted = internalModel.isHiddenFromRecordArrays();
+    let recordArrays = internalModel._recordArrays;
+
+    if (!isDeleted && !internalModel.isEmpty()) {
+      if (!recordArrays.has(array)) {
+        modelsToAdd.push(internalModel);
+        recordArrays.add(array);
+      }
+    }
+
+    if (isDeleted) {
+      modelsToRemove.push(internalModel);
+      recordArrays.delete(array)
+    }
+  }
+
+  if (modelsToAdd.length > 0)    { array._pushInternalModels(modelsToAdd); }
+  if (modelsToRemove.length > 0) { array._removeInternalModels(modelsToRemove); }
+}
+
+function removeFromAdapterPopulatedRecordArrays(internalModels) {
+  for (let i = 0; i < internalModels.length; i++) {
+    let internalModel = internalModels[i];
+    let list = internalModel._recordArrays.list;
+
+    for (let j = 0; j < list.length; j++) {
+      // TODO: group by arrays, so we can batch remove
+      list[j]._removeInternalModels([internalModel]);
+    }
+
+    internalModel._recordArrays.clear();
+  }
+}
+
+export function associateWithRecordArray(internalModels, array) {
+  for (let i = 0, l = internalModels.length; i < l; i++) {
+    let internalModel = internalModels[i];
+    internalModel._recordArrays.add(array);
+  }
+}
diff --git a/addon/-private/system/record-arrays/adapter-populated-record-array.js b/addon/-private/system/record-arrays/adapter-populated-record-array.js
@@ -1,6 +1,7 @@
 import Ember from 'ember';
 import RecordArray from "./record-array";
 import cloneNull from "../clone-null";
+import { associateWithRecordArray } from '../record-array-manager';
 
 /**
   @module ember-data
@@ -87,11 +88,7 @@ export default RecordArray.extend({
       links: cloneNull(payload.links)
     });
 
-    for (let i = 0, l = internalModels.length; i < l; i++) {
-      let internalModel = internalModels[i];
-
-      internalModel._recordArrays.add(this);
-    }
+    associateWithRecordArray(internalModels, this);
 
     // TODO: should triggering didLoad event be the last action of the runLoop?
     Ember.run.once(this, 'trigger', 'didLoad');