Skip to content

Commit

Permalink
[BUGFIX beta] Observers after Dependent Key Change
Browse files Browse the repository at this point in the history
Currently when a property is overriden in an extended class and its
dependent keys change, observers watching that property will still fire
for changes made to the old set of dependent keys.

```js
var count = 0;

var MyClass = Ember.Object.extend({
    prop1: 'foo',
    content: Ember.computed.alias('prop1'),
    incrementCount: Ember.observer('content', function () { count++; })
});

var obj = MyClass.extend({
    prop2: Ember.computed.alias('prop1'),
    content: Ember.computed.alias('prop2') // redefine content with diff depKey
}).create();

Ember.run(function() {
    obj.set('prop1', 'bar');
});

// assert(count === 1); // Fails, should be 1 but in fact is 2
```
  • Loading branch information
mmpestorich committed Aug 6, 2015
1 parent af8ed04 commit 75a7ba3
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
3 changes: 3 additions & 0 deletions packages/ember-metal/lib/property_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ function iterDeps(method, obj, deps, depKey, seen, meta) {
keys = keysOf(deps);
for (i = 0; i < keys.length; i++) {
key = keys[i];

if (!deps[key]) { continue; }

possibleDesc = obj[key];
desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined;

Expand Down
36 changes: 36 additions & 0 deletions packages/ember-metal/tests/alias_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,42 @@ QUnit.test('basic lifecycle', function() {
equal(m.readableDeps('foo.faz').bar, 0);
});

QUnit.test('old dependent keys should not trigger property changes', function() {
var obj1 = Object.create(null);
defineProperty(obj1, 'foo', null, null);
defineProperty(obj1, 'bar', alias('foo'));
defineProperty(obj1, 'baz', alias('foo'));
defineProperty(obj1, 'baz', alias('bar')); // redefine baz
addObserver(obj1, 'baz', incrementCount);

set(obj1, 'foo', 'FOO');
equal(count, 1);

removeObserver(obj1, 'baz', incrementCount);

set(obj1, 'foo', 'OOF');
equal(count, 1);
});

QUnit.test('overridden dependent keys should not trigger property changes', function() {
var obj1 = Object.create(null);
defineProperty(obj1, 'foo', null, null);
defineProperty(obj1, 'bar', alias('foo'));
defineProperty(obj1, 'baz', alias('foo'));
addObserver(obj1, 'baz', incrementCount);

var obj2 = Object.create(obj1);
defineProperty(obj2, 'baz', alias('bar')); // override baz

set(obj2, 'foo', 'FOO');
equal(count, 1);

removeObserver(obj2, 'baz', incrementCount);

set(obj2, 'foo', 'OOF');
equal(count, 1);
});

QUnit.test('begins watching alt key as soon as alias is watched', function() {
defineProperty(obj, 'bar', alias('foo.faz'));
addObserver(obj, 'bar', incrementCount);
Expand Down

0 comments on commit 75a7ba3

Please sign in to comment.