From b3e1d67899b06b051a0cc2c12635998215ad6621 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Fri, 11 Nov 2016 12:54:57 -0800 Subject: [PATCH] [BUGFIX #4649 #4648] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * no longer invalid relationships property when accessing it the first time #4649 * no longer duplicate “notifyHasManyChange” when adding a record fixes #4648 No longer warn/error in glimmer 2 by accidentally causing duplicate work. --- addon/-private/system/many-array.js | 15 +++++++-------- tests/unit/model/relationships/has-many-test.js | 17 ++++++++++++++++- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/addon/-private/system/many-array.js b/addon/-private/system/many-array.js index 8501010f9df..a79d590ee24 100644 --- a/addon/-private/system/many-array.js +++ b/addon/-private/system/many-array.js @@ -129,7 +129,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { this.relationship = this.relationship || null; this.currentState = Ember.A([]); - this.flushCanonical(); + this.flushCanonical(false); }, objectAt(index) { @@ -141,7 +141,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { return this.currentState[index].getRecord(); }, - flushCanonical() { + flushCanonical(isInitialized = true) { let toSet = this.canonicalState; //a hack for not removing new records @@ -161,8 +161,11 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { } this.currentState = toSet; this.arrayContentDidChange(0, oldLength, this.length); - //TODO Figure out to notify only on additions and maybe only if unloaded - this.relationship.notifyHasManyChanged(); + + if (isInitialized) { + //TODO Figure out to notify only on additions and maybe only if unloaded + this.relationship.notifyHasManyChanged(); + } }, internalReplace(idx, amt, objects) { @@ -173,10 +176,6 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { this.currentState.splice.apply(this.currentState, [idx, amt].concat(objects)); this.set('length', this.currentState.length); this.arrayContentDidChange(idx, amt, objects.length); - if (objects) { - //TODO(Igor) probably needed only for unloaded records - this.relationship.notifyHasManyChanged(); - } }, //TODO(Igor) optimize diff --git a/tests/unit/model/relationships/has-many-test.js b/tests/unit/model/relationships/has-many-test.js index db8d92ebb34..57bf2f9dabb 100644 --- a/tests/unit/model/relationships/has-many-test.js +++ b/tests/unit/model/relationships/has-many-test.js @@ -770,9 +770,13 @@ test("DS.hasMany is async by default", function(assert) { }); test("DS.ManyArray is lazy", function(assert) { + let peopleDidChange = 0; let Tag = DS.Model.extend({ name: DS.attr('string'), - people: DS.hasMany('person') + people: DS.hasMany('person'), + peopleDidChange: Ember.observer('people', function() { + peopleDidChange++; + }) }); let Person = DS.Model.extend({ @@ -787,9 +791,20 @@ test("DS.ManyArray is lazy", function(assert) { let hasManyRelationship = tag.hasMany('people').hasManyRelationship; assert.ok(!hasManyRelationship._manyArray); run(function() { + assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (before access)'); tag.get('people'); + assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (sync after access)'); }); + assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (after access, but after the current run loop)'); assert.ok(hasManyRelationship._manyArray instanceof DS.ManyArray); + + let person = Ember.run(() => env.store.createRecord('person')); + + Ember.run(() => { + assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (before access)'); + tag.get('people').addObject(person); + assert.equal(peopleDidChange, 1, 'expect people hasMany to have changed exactly once'); + }); }); testInDebug("throws assertion if of not set with an array", function(assert) {