Skip to content

Commit

Permalink
Data Adapter: Only trigger model type update if the record live array…
Browse files Browse the repository at this point in the history
… count actually changed

Fixes emberjs/ember-inspector#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.

(cherry picked from commit 7bd47da)
  • Loading branch information
teddyzeenny authored and rwjblue committed Oct 4, 2017
1 parent 3727e2e commit 96d010b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
8 changes: 6 additions & 2 deletions packages/ember-extension-support/lib/data_adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
};
Expand Down
27 changes: 27 additions & 0 deletions packages/ember-extension-support/tests/data_adapter_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 96d010b

Please sign in to comment.