From 7bd47dacd3a761a98d9c78ec22546584e62bc621 Mon Sep 17 00:00:00 2001 From: Teddy Zeenny Date: Sun, 27 Aug 2017 16:35:58 +0300 Subject: [PATCH] Data Adapter: Only trigger model type update if the record live array count actually changed Fixes https://github.com/emberjs/ember-inspector/issues/690 The issue: In some cases, Ember Data's `peekAll` triggers `arrayContentDidChange` on the record array manager's live record array even if the records didn't change. One example case is when we call `unloadRecord` and then `peekAll` within the same run loop. The `peekAll` will trigger the array observer's `didChange` callback (with zero changes). This is generally harmless as long as we guard against that in our data adapter. Without the guard, we risk causing an infinite recursion because our `didChange` observer itself calls `peekAll`, which in the above scenario will re-trigger the observer and so on. --- .../lib/data_adapter.js | 8 ++++-- .../tests/data_adapter_test.js | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/ember-extension-support/lib/data_adapter.js b/packages/ember-extension-support/lib/data_adapter.js index a7e1afb3c5d..8772dff5047 100644 --- a/packages/ember-extension-support/lib/data_adapter.js +++ b/packages/ember-extension-support/lib/data_adapter.js @@ -294,8 +294,12 @@ export default EmberObject.extend({ } let observer = { - didChange() { - run.scheduleOnce('actions', this, onChange); + didChange(array, idx, removedCount, addedCount) { + // Only re-fetch records if the record count changed + // (which is all we care about as far as model types are concerned). + if (removedCount > 0 || addedCount > 0) { + run.scheduleOnce('actions', this, onChange); + } }, willChange() { return this; } }; diff --git a/packages/ember-extension-support/tests/data_adapter_test.js b/packages/ember-extension-support/tests/data_adapter_test.js index 7e526ef6213..1acbc4450dc 100644 --- a/packages/ember-extension-support/tests/data_adapter_test.js +++ b/packages/ember-extension-support/tests/data_adapter_test.js @@ -140,6 +140,33 @@ moduleFor('Data Adapter', class extends ApplicationTestCase { }); } + ['@test Model Types Updated but Unchanged Do not Trigger Callbacks']() { + expect(0); + let records = emberA([1, 2, 3]); + this.add('data-adapter:main', DataAdapter.extend({ + getRecords(klass, name) { + return records; + } + })); + this.add('model:post', PostClass); + + return this.visit('/').then(() => { + adapter = this.applicationInstance.lookup('data-adapter:main'); + + function modelTypesAdded(types) { + run(() => { + records.arrayContentDidChange(0, 0, 0); + }); + } + + function modelTypesUpdated(types) { + ok(false, "modelTypesUpdated should not be triggered if the array didn't change"); + } + + adapter.watchModelTypes(modelTypesAdded, modelTypesUpdated); + }); + } + ['@test Records Added']() { let countAdded = 1; let post = PostClass.create();