From a2fc1eeec122b8eaa3fdc985877bb057b7017d05 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 29 Aug 2015 23:17:11 -0400 Subject: [PATCH] [BUGFIX canary] Trust Ember.set This change eliminates unnecessary dirtying of proxy streams when the source object is an Ember.Object. It results in significant re-render performance improvement in certain scenarios, especially when people are iterating over lists of Ember.Objects to render components with expensive lifecycle hooks. We already have a longstanding convention that people need to use `set` to mutate Ember.Objects if they expect any observability, and as long as people are using `set`, interior mutability will be captured by other observers further downstream. The same is not necessarily true for POJOs. We would like to allow something conceptually like: myComponent.set('thing', aPojo); aPojo.deep.inside.here = 'newValue'; myComponent.notifyPropertyChange('thing'); to work correctly, such that the deep mutation gets noticed. Which is why this change treats POJOs and Ember.Object's differently. --- packages/ember-metal/lib/streams/dependency.js | 2 ++ packages/ember-metal/lib/streams/proxy-stream.js | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/ember-metal/lib/streams/dependency.js b/packages/ember-metal/lib/streams/dependency.js index 950e5b16032..de169afdc90 100644 --- a/packages/ember-metal/lib/streams/dependency.js +++ b/packages/ember-metal/lib/streams/dependency.js @@ -49,7 +49,9 @@ merge(Dependency.prototype, { this.unsubscribe(); this.subscribe(); } + return true; } + return false; }, getValue() { diff --git a/packages/ember-metal/lib/streams/proxy-stream.js b/packages/ember-metal/lib/streams/proxy-stream.js index ed1bd356498..9f8ffefdbb1 100644 --- a/packages/ember-metal/lib/streams/proxy-stream.js +++ b/packages/ember-metal/lib/streams/proxy-stream.js @@ -1,5 +1,6 @@ import merge from 'ember-metal/merge'; import Stream from 'ember-metal/streams/stream'; +import EmberObject from 'ember-runtime/system/object'; function ProxyStream(source, label) { this.init(label); @@ -18,8 +19,13 @@ merge(ProxyStream.prototype, { }, setSource(source) { - this.sourceDep.replace(source); - this.notify(); + let didChange = this.sourceDep.replace(source); + if (didChange || !(source instanceof EmberObject)) { + // If the source changed, we must notify. If the source is not + // an Ember.Object, we must also notify, because it could have + // interior mutability that is otherwise not being observed. + this.notify(); + } } });