Skip to content

Commit

Permalink
fix(AccessKeyed): avoid dirty-checking keyed array access
Browse files Browse the repository at this point in the history
fixes #289
  • Loading branch information
jdanyow committed Jan 23, 2016
1 parent eff3162 commit 7d01567
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 6 deletions.
5 changes: 4 additions & 1 deletion src/ast.js
Expand Up @@ -317,7 +317,10 @@ export class AccessKeyed extends Expression {
if (obj instanceof Object) {
this.key.connect(binding, scope);
let key = this.key.evaluate(scope);
if (key !== null && key !== undefined) {
// observe the property represented by the key as long as it's not an array
// being accessed by an integer key which would require dirty-checking.
if (key !== null && key !== undefined
&& !(Array.isArray(obj) && typeof(key) === 'number')) {
binding.observeProperty(obj, key);
}
}
Expand Down
10 changes: 6 additions & 4 deletions test/access-keyed-observer.spec.js
Expand Up @@ -267,10 +267,11 @@ describe('AccessKeyedObserver', () => {
expect(el.value).toBe(obj.array[obj.key]);
});

it('responds to property change', done => {
it('does not respond to property change', done => {
let original = el.value;
obj.array[obj.key] = 'foo';
setTimeout(() => {
expect(el.value).toBe(obj.array[obj.key]);
expect(el.value).toBe(original);
done();
}, checkDelay * 2);
});
Expand Down Expand Up @@ -399,10 +400,11 @@ describe('AccessKeyedObserver', () => {
expect(el.value).toBe(obj.array[obj.key]);
});

it('responds to property change', done => {
it('does not respond to property change', done => {
let original = el.value;
obj.array[obj.key] = 'foo';
setTimeout(() => {
expect(el.value).toBe(obj.array[obj.key]);
expect(el.value).toBe(original);
done();
}, checkDelay * 2);
});
Expand Down
24 changes: 23 additions & 1 deletion test/ast/access-keyed.spec.js
@@ -1,4 +1,4 @@
import {AccessKeyed, AccessScope, LiteralString} from '../../src/ast';
import {AccessKeyed, AccessScope, LiteralString, LiteralPrimitive} from '../../src/ast';
import {createScopeForTest} from '../../src/scope';

describe('AccessKeyed', () => {
Expand Down Expand Up @@ -40,4 +40,26 @@ describe('AccessKeyed', () => {
scope = createScopeForTest({});
expect(expression.evaluate(scope, null)).toBe(undefined);
});

it('does not observes property in keyed object access when key is number', () => {
let scope = createScopeForTest({ foo: { '0': 'hello world' } });
let expression = new AccessKeyed(new AccessScope('foo', 0), new LiteralPrimitive(0));
expect(expression.evaluate(scope, null)).toBe('hello world');
let binding = { observeProperty: jasmine.createSpy('observeProperty') };
expression.connect(binding, scope);
expect(binding.observeProperty.calls.argsFor(0)).toEqual([scope.bindingContext, 'foo']);
expect(binding.observeProperty.calls.argsFor(1)).toEqual([scope.bindingContext.foo, 0]);
expect(binding.observeProperty.calls.count()).toBe(2);
});

it('does not observe property in keyed array access when key is number', () => {
let scope = createScopeForTest({ foo: ['hello world'] });
let expression = new AccessKeyed(new AccessScope('foo', 0), new LiteralPrimitive(0));
expect(expression.evaluate(scope, null)).toBe('hello world');
let binding = { observeProperty: jasmine.createSpy('observeProperty') };
expression.connect(binding, scope);
expect(binding.observeProperty.calls.argsFor(0)).toEqual([scope.bindingContext, 'foo']);
expect(binding.observeProperty).not.toHaveBeenCalledWith(scope.bindingContext.foo, 0);
expect(binding.observeProperty.calls.count()).toBe(1);
});
});

0 comments on commit 7d01567

Please sign in to comment.