Skip to content

Commit

Permalink
[PERF beta] @each should remain a stable node for chains.
Browse files Browse the repository at this point in the history
`@each` was designed for chaining, the special cased eager behavior + it invalidating for array changes meant that the EachProxy and chains were rebuilt every array change, instead of the leaves just changing.
Conflicts:
	packages/ember-runtime/lib/system/each_proxy.js
	packages/ember-runtime/tests/mixins/array_test.js
  • Loading branch information
krisselden committed Aug 6, 2015
1 parent af8ed04 commit 5868df3
Show file tree
Hide file tree
Showing 19 changed files with 116 additions and 262 deletions.
17 changes: 1 addition & 16 deletions packages/ember-metal/lib/chains.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,6 @@ function ChainNode(parent, key, value) {
addChainWatcher(this._object, this._key, this);
}
}

// Special-case: the EachProxy relies on immediate evaluation to
// establish its observers.
//
// TODO: Replace this with an efficient callback that the EachProxy
// can implement.
if (this._parent && this._parent._key === '@each') {
this.value();
}
}

function lazyGet(obj, key) {
Expand All @@ -216,7 +207,7 @@ function lazyGet(obj, key) {
}

// Use `get` if the return value is an EachProxy or an uncacheable value.
if (key === '@each' || isVolatile(obj[key])) {
if (isVolatile(obj[key])) {
return get(obj, key);
// Otherwise attempt to get the cached value of the computed property
} else {
Expand Down Expand Up @@ -374,12 +365,6 @@ ChainNode.prototype = {
addChainWatcher(obj, this._key, this);
}
this._value = undefined;

// Special-case: the EachProxy relies on immediate evaluation to
// establish its observers.
if (this._parent && this._parent._key === '@each') {
this.value();
}
}

// then notify chains...
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-metal/lib/expand_properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var SPLIT_REGEX = /\{|\}/;
Ember.expandProperties('{foo,bar}', echo); //=> 'foo', 'bar'
Ember.expandProperties('foo.{bar,baz}', echo); //=> 'foo.bar', 'foo.baz'
Ember.expandProperties('{foo,bar}.baz', echo); //=> 'foo.baz', 'bar.baz'
Ember.expandProperties('foo.{bar,baz}.@each', echo) //=> 'foo.bar.@each', 'foo.baz.@each'
Ember.expandProperties('foo.{bar,baz}.[]', echo) //=> 'foo.bar.[]', 'foo.baz.[]'
Ember.expandProperties('{foo,bar}.{spam,eggs}', echo) //=> 'foo.spam', 'foo.eggs', 'bar.spam', 'bar.eggs'
Ember.expandProperties('{foo}.bar.{baz}') //=> 'foo.bar.baz'
```
Expand Down
5 changes: 3 additions & 2 deletions packages/ember-metal/tests/expand_properties_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ QUnit.test('Properties without expansions are unaffected', function() {

expandProperties('a', addProperty);
expandProperties('a.b', addProperty);
expandProperties('a.b.@each', addProperty);
expandProperties('a.b.[]', addProperty);
expandProperties('[email protected]', addProperty);

deepEqual(['a', 'a.b', 'a.b.@each'].sort(), foundProperties.sort());
deepEqual(['a', 'a.b', 'a.b.[]', 'a.b.@each.c'].sort(), foundProperties.sort());
});

QUnit.test('A single expansion at the end expands properly', function() {
Expand Down
27 changes: 12 additions & 15 deletions packages/ember-runtime/lib/mixins/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
sendEvent,
hasListeners
} from 'ember-metal/events';
import { isWatching } from 'ember-metal/watching';
import EachProxy from 'ember-runtime/system/each_proxy';

function arrayObserversHelper(obj, target, opts, operation, notify) {
var willChange = (opts && opts.willChange) || 'arrayWillChange';
Expand Down Expand Up @@ -422,13 +422,13 @@ export default Mixin.create(Enumerable, {
}
}

// Make sure the @each proxy is set up if anyone is observing @each
if (isWatching(this, '@each')) {
get(this, '@each');
if (this.__each) {
this.__each.arrayWillChange(this, startIdx, removeAmt, addAmt);
}

sendEvent(this, '@array:before', [this, startIdx, removeAmt, addAmt]);


if (startIdx >= 0 && removeAmt >= 0 && get(this, 'hasEnumerableObservers')) {
removing = [];
lim = startIdx + removeAmt;
Expand Down Expand Up @@ -489,6 +489,11 @@ export default Mixin.create(Enumerable, {
}

this.enumerableContentDidChange(removeAmt, adding);

if (this.__each) {
this.__each.arrayDidChange(this, startIdx, removeAmt, addAmt);
}

sendEvent(this, '@array:change', [this, startIdx, removeAmt, addAmt]);

var length = get(this, 'length');
Expand All @@ -508,10 +513,6 @@ export default Mixin.create(Enumerable, {
return this;
},

// ..........................................................
// ENUMERATED PROPERTIES
//

/**
Returns a special object that can be used to observe individual properties
on the array. Just get an equivalent property on this object and it will
Expand All @@ -525,15 +526,11 @@ export default Mixin.create(Enumerable, {
@public
*/
'@each': computed(function() {
// TODO use Symbol or add to meta
if (!this.__each) {
// ES6TODO: GRRRRR
var EachProxy = requireModule('ember-runtime/system/each_proxy')['EachProxy'];

this.__each = new EachProxy({
content: this
});
this.__each = new EachProxy(this);
}

return this.__each;
})
}).volatile()
});
Loading

0 comments on commit 5868df3

Please sign in to comment.