From b0c9d05ea7bf804f59020bcb25e34134a94123b9 Mon Sep 17 00:00:00 2001 From: Rado Kirov Date: Mon, 27 Oct 2014 23:16:31 -0700 Subject: [PATCH] feat(view): add onChange implementation to view. --- .../element_injector/instantiate_benchmark.js | 4 +- .../instantiate_benchmark_codegen.js | 2 +- .../instantiate_directive_benchmark.js | 2 +- modules/core/src/compiler/element_injector.js | 36 ++++++-- modules/core/src/compiler/view.js | 50 +++++++---- .../test/compiler/element_injector_spec.js | 34 +++++--- modules/core/test/compiler/view_spec.js | 86 ++++++++++++++++--- 7 files changed, 162 insertions(+), 52 deletions(-) diff --git a/modules/benchmarks/src/element_injector/instantiate_benchmark.js b/modules/benchmarks/src/element_injector/instantiate_benchmark.js index 2e3dc9367b..0aba57230d 100644 --- a/modules/benchmarks/src/element_injector/instantiate_benchmark.js +++ b/modules/benchmarks/src/element_injector/instantiate_benchmark.js @@ -8,7 +8,7 @@ export function run () { var appInjector = new Injector([]); var bindings = [A, B, C]; - var proto = new ProtoElementInjector(null, bindings, []); + var proto = new ProtoElementInjector(null, bindings, [], false); for (var i = 0; i < ITERATIONS; ++i) { var ei = proto.instantiate({view:null}); ei.instantiateDirectives(appInjector); @@ -31,4 +31,4 @@ class C { constructor(a:A, b:B) { count++; } -} \ No newline at end of file +} diff --git a/modules/benchmarks/src/element_injector/instantiate_benchmark_codegen.js b/modules/benchmarks/src/element_injector/instantiate_benchmark_codegen.js index 00fe76c4b1..e810769fb6 100644 --- a/modules/benchmarks/src/element_injector/instantiate_benchmark_codegen.js +++ b/modules/benchmarks/src/element_injector/instantiate_benchmark_codegen.js @@ -16,7 +16,7 @@ export function run () { ], false)]; - var proto = new ProtoElementInjector(null, bindings, []); + var proto = new ProtoElementInjector(null, bindings, [], false); for (var i = 0; i < ITERATIONS; ++i) { var ei = proto.instantiate({view:null}); ei.instantiateDirectives(appInjector); diff --git a/modules/benchmarks/src/element_injector/instantiate_directive_benchmark.js b/modules/benchmarks/src/element_injector/instantiate_directive_benchmark.js index 29591f4215..7cca359d45 100644 --- a/modules/benchmarks/src/element_injector/instantiate_directive_benchmark.js +++ b/modules/benchmarks/src/element_injector/instantiate_directive_benchmark.js @@ -8,7 +8,7 @@ export function run () { var appInjector = new Injector([]); var bindings = [A, B, C]; - var proto = new ProtoElementInjector(null, bindings, []); + var proto = new ProtoElementInjector(null, bindings, [], false); var ei = proto.instantiate({view:null}); for (var i = 0; i < ITERATIONS; ++i) { diff --git a/modules/core/src/compiler/element_injector.js b/modules/core/src/compiler/element_injector.js index 69ddbe5071..0efa33e46f 100644 --- a/modules/core/src/compiler/element_injector.js +++ b/modules/core/src/compiler/element_injector.js @@ -118,7 +118,6 @@ export class ProtoElementInjector extends TreeNode { query_keyId1:int; textNodes:List; - hasProperties:boolean; events:Map; elementInjector:ElementInjector; @@ -144,8 +143,10 @@ export class ProtoElementInjector extends TreeNode { @FIELD('_key7:int') @FIELD('_key8:int') @FIELD('_key9:int') - @FIELD('textNodes:List') - constructor(parent:ProtoElementInjector, bindings:List, textNodes:List) { + @FIELD('textNodeIndices:List') + @FIELD('hasElementPropertyBindings:bool') + constructor(parent:ProtoElementInjector, bindings:List, textNodeIndices:List, + hasElementPropertyBindings:boolean) { super(parent); this._elementInjector = null; @@ -177,10 +178,8 @@ export class ProtoElementInjector extends TreeNode { throw 'Maximum number of directives per element has been reached.'; } - this.textNodes = textNodes; - - // dummy fields to make analyzer happy - this.hasProperties = false; + this.textNodeIndices = textNodeIndices; + this.hasElementPropertyBindings = hasElementPropertyBindings; } instantiate({view}):ElementInjector { @@ -418,5 +417,28 @@ export class ElementInjector extends TreeNode { if (p._keyId9 === keyId) return this._obj9; return _undefined; } + + getAtIndex(index:int) { + if (index == 0) return this._obj0; + if (index == 1) return this._obj1; + if (index == 2) return this._obj2; + if (index == 3) return this._obj3; + if (index == 4) return this._obj4; + if (index == 5) return this._obj5; + if (index == 6) return this._obj6; + if (index == 7) return this._obj7; + if (index == 8) return this._obj8; + if (index == 9) return this._obj9; + throw new OutOfBoundsAccess(index); + } } +class OutOfBoundsAccess extends Error { + constructor(index) { + this.message = `Index ${index} is out-of-bounds.`; + } + + toString() { + return this.message; + } +} diff --git a/modules/core/src/compiler/view.js b/modules/core/src/compiler/view.js index d4e19b5025..beac7e0fe3 100644 --- a/modules/core/src/compiler/view.js +++ b/modules/core/src/compiler/view.js @@ -23,14 +23,15 @@ export class View { /// to keep track of the nodes. @FIELD('final nodes:List') @FIELD('final onChangeDispatcher:OnChangeDispatcher') - constructor(fragment:DocumentFragment, elementInjector:List, rootElementInjectors:List, textNodes:List) { + constructor(fragment:DocumentFragment, elementInjector:List, + rootElementInjectors:List, textNodes:List, bindElements:List) { this.fragment = fragment; this.nodes = ListWrapper.clone(fragment.childNodes); this.elementInjectors = elementInjector; this.rootElementInjectors = rootElementInjectors; this.onChangeDispatcher = null; this.textNodes = textNodes; - this.bindElements = null; + this.bindElements = bindElements; } onRecordChange(record:Record, target) { @@ -39,7 +40,7 @@ export class View { // we know that it is DirectivePropertyMemento var directiveMemento:DirectivePropertyMemento = target; directiveMemento.invoke(record, this.elementInjectors); - } else if (target instanceof ElementPropertyMemento) { + } else if (target instanceof ElementPropertyMemento) { var elementMemento:ElementPropertyMemento = target; elementMemento.invoke(record, this.bindElements); } else { @@ -83,8 +84,10 @@ export class ProtoView { var elementInjectors = ProtoView._createElementInjectors(elements, protos); var rootElementInjectors = ProtoView._rootElementInjectors(elementInjectors); var textNodes = ProtoView._textNodes(elements, protos); + var bindElements = ProtoView._bindElements(elements, protos); - return new View(fragment, elementInjectors, rootElementInjectors, textNodes); + return new View(fragment, elementInjectors, rootElementInjectors, textNodes, + bindElements); } static _createElementInjectors(elements, protos) { @@ -92,7 +95,11 @@ export class ProtoView { for (var i = 0; i < protos.length; ++i) { injectors[i] = ProtoView._createElementInjector(elements[i], protos[i]); } - ListWrapper.forEach(protos, p => p.clearElementInjector()); + // Cannot be rolled into loop above, because parentInjector pointers need + // to be set on the children. + for (var i = 0; i < protos.length; ++i) { + protos[i].clearElementInjector(); + } return injectors; } @@ -108,16 +115,26 @@ export class ProtoView { static _textNodes(elements, protos) { var textNodes = []; for (var i = 0; i < protos.length; ++i) { - ProtoView._collectTextNodes(textNodes, elements[i], protos[i]); + ProtoView._collectTextNodes(textNodes, elements[i], + protos[i].textNodeIndices); } return textNodes; } - static _collectTextNodes(allTextNodes, element, proto) { + static _bindElements(elements, protos):List { + var bindElements = []; + for (var i = 0; i < protos.length; ++i) { + if (protos[i].hasElementPropertyBindings) ListWrapper.push( + bindElements, elements[i]); + } + return bindElements; + } + + static _collectTextNodes(allTextNodes, element, indices) { var childNodes = DOM.childNodes(element); - ListWrapper.forEach(proto.textNodes, (i) => { - ListWrapper.push(allTextNodes, childNodes[i]); - }); + for (var i = 0; i < indices.length; ++i) { + ListWrapper.push(allTextNodes, childNodes[indices[i]]); + } } } @@ -129,8 +146,8 @@ export class ElementPropertyMemento { this._propertyName = propertyName; } - invoke(record:Record, elementInjectors:List) { - var element:Element = elementInjectors[this._elementIndex]; + invoke(record:Record, bindElements:List) { + var element:Element = bindElements[this._elementIndex]; DOM.setProperty(element, this._propertyName, record.currentValue); } } @@ -138,14 +155,13 @@ export class ElementPropertyMemento { export class DirectivePropertyMemento { @FIELD('final _elementInjectorIndex:int') @FIELD('final _directiveIndex:int') - @FIELD('final _setterName:String') + @FIELD('final _setterName:string') @FIELD('final _setter:SetterFn') constructor( elementInjectorIndex:number, directiveIndex:number, - setterName:String, - setter:SetterFn) - { + setterName:string, + setter:SetterFn) { this._elementInjectorIndex = elementInjectorIndex; this._directiveIndex = directiveIndex; this._setterName = setterName; @@ -154,7 +170,7 @@ export class DirectivePropertyMemento { invoke(record:Record, elementInjectors:List) { var elementInjector:ElementInjector = elementInjectors[this._elementInjectorIndex]; - var directive = elementInjectors[this._directiveIndex]; + var directive = elementInjector.getAtIndex(this._directiveIndex); this._setter(directive, record.currentValue); } } diff --git a/modules/core/test/compiler/element_injector_spec.js b/modules/core/test/compiler/element_injector_spec.js index c0a4efadbf..35768374ac 100644 --- a/modules/core/test/compiler/element_injector_spec.js +++ b/modules/core/test/compiler/element_injector_spec.js @@ -62,7 +62,7 @@ export function main() { if (isBlank(appInjector)) appInjector = new Injector([]); if (isBlank(props)) props = {}; - var proto = new ProtoElementInjector(null, bindings, []); + var proto = new ProtoElementInjector(null, bindings, [], false); var inj = proto.instantiate({view: props["view"]}); inj.instantiateDirectives(appInjector); return inj; @@ -71,11 +71,12 @@ export function main() { function parentChildInjectors(parentBindings, childBindings) { var inj = new Injector([]); - var protoParent = new ProtoElementInjector(null, parentBindings, []); + var protoParent = new ProtoElementInjector(null, parentBindings, [], false); var parent = protoParent.instantiate({view: null}); parent.instantiateDirectives(inj); - var protoChild = new ProtoElementInjector(protoParent, childBindings, []); + var protoChild = new ProtoElementInjector(protoParent, childBindings, [], + false); var child = protoChild.instantiate({view: null}); child.instantiateDirectives(inj); @@ -85,9 +86,9 @@ export function main() { describe("ElementInjector", function () { describe("proto injectors", function () { it("should construct a proto tree", function () { - var p = new ProtoElementInjector(null, [], []); - var c1 = new ProtoElementInjector(p, [], []); - var c2 = new ProtoElementInjector(p, [], []); + var p = new ProtoElementInjector(null, [], [], false); + var c1 = new ProtoElementInjector(p, [], [], false); + var c2 = new ProtoElementInjector(p, [], [], false); expect(humanize(p, [ [p, 'parent'], @@ -99,9 +100,9 @@ export function main() { describe("instantiate", function () { it("should create an element injector", function () { - var protoParent = new ProtoElementInjector(null, [], []); - var protoChild1 = new ProtoElementInjector(protoParent, [], []); - var protoChild2 = new ProtoElementInjector(protoParent, [], []); + var protoParent = new ProtoElementInjector(null, [], [], false); + var protoChild1 = new ProtoElementInjector(protoParent, [], [], false); + var protoChild2 = new ProtoElementInjector(protoParent, [], [], false); var p = protoParent.instantiate({view: null}); var c1 = protoChild1.instantiate({view: null}); @@ -117,12 +118,12 @@ export function main() { describe("hasBindings", function () { it("should be true when there are bindings", function () { - var p = new ProtoElementInjector(null, [Directive], []); + var p = new ProtoElementInjector(null, [Directive], [], false); expect(p.hasBindings).toBeTruthy(); }); it("should be false otherwise", function () { - var p = new ProtoElementInjector(null, [], []); + var p = new ProtoElementInjector(null, [], [], false); expect(p.hasBindings).toBeFalsy(); }); }); @@ -196,6 +197,15 @@ export function main() { var inj = injector([bind(Directive).toClass(Directive)]); expect(inj.get(Directive)).toBeAnInstanceOf(Directive); }); + + it("should allow for direct access using getAtIndex", function () { + var inj = injector([bind(Directive).toClass(Directive)]); + expect(inj.getAtIndex(0)).toBeAnInstanceOf(Directive); + expect(() => inj.getAtIndex(-1)).toThrowError( + 'Index -1 is out-of-bounds.'); + expect(() => inj.getAtIndex(10)).toThrowError( + 'Index 10 is out-of-bounds.'); + }); }); describe("special objects", function () { @@ -207,4 +217,4 @@ export function main() { }); }); }); -} \ No newline at end of file +} diff --git a/modules/core/test/compiler/view_spec.js b/modules/core/test/compiler/view_spec.js index 0a28d73f3f..109a1cc849 100644 --- a/modules/core/test/compiler/view_spec.js +++ b/modules/core/test/compiler/view_spec.js @@ -1,28 +1,36 @@ -import {describe, xit, it, expect} from 'test_lib/test_lib'; -import {ProtoView} from 'core/compiler/view'; +import {describe, xit, it, expect, beforeEach} from 'test_lib/test_lib'; +import {ProtoView, ElementPropertyMemento, DirectivePropertyMemento} from 'core/compiler/view'; +import {Record} from 'change_detection/record'; import {ProtoElementInjector, ElementInjector} from 'core/compiler/element_injector'; import {DOM, Element} from 'facade/dom'; +import {FIELD} from 'facade/lang'; class Directive { + @FIELD('prop') + constructor() { + this.prop = 'foo'; + } } export function main() { describe('view', function() { - describe('ProtoView', function() { - it('should create view instance and locate basic parts', function() { - var template = DOM.createTemplate( + var tempalteWithThreeTypesOfBindings = '
' + 'Hello {}!' + '
' + - 'don\'t show me' + + 'don\'t show me' + '
' + - '
'); + ''; + + describe('ProtoView', function() { + it('should create view instance and locate basic parts', function() { + var template = DOM.createTemplate(tempalteWithThreeTypesOfBindings); var diBindings = []; - var sectionPI = new ProtoElementInjector(null, [], [0]); - var divPI = new ProtoElementInjector(sectionPI, [Directive], []); - var spanPI = new ProtoElementInjector(divPI, [], []); + var sectionPI = new ProtoElementInjector(null, [], [0], false); + var divPI = new ProtoElementInjector(sectionPI, [Directive], [], false); + var spanPI = new ProtoElementInjector(divPI, [], [], true); var protoElementInjectors = [sectionPI, divPI, spanPI]; var protoWatchGroup = null; @@ -42,6 +50,7 @@ export function main() { expect(view.elementInjectors[2]).toBeNull(); expect(view.textNodes.length).toEqual(1); + expect(view.bindElements.length).toEqual(1); expect(view.textNodes[0].nodeValue).toEqual('Hello {}!'); }); @@ -51,14 +60,67 @@ export function main() { '
' + ''); - var sectionPI = new ProtoElementInjector(null, [Directive], []); - var divPI = new ProtoElementInjector(sectionPI, [Directive], []); + var sectionPI = new ProtoElementInjector(null, [Directive], [], false); + var divPI = new ProtoElementInjector(sectionPI, [Directive], [], false); var pv = new ProtoView(template, [], [sectionPI, divPI], null, false); var view = pv.instantiate(); expect(view.rootElementInjectors.length).toEqual(1); }); + + describe('react to watch group changes', function() { + var view; + beforeEach(() => { + var template = DOM.createTemplate(tempalteWithThreeTypesOfBindings); + + var diBindings = []; + + var sectionPI = new ProtoElementInjector(null, [], [0], false); + var divPI = new ProtoElementInjector(sectionPI, [Directive], [], false); + var spanPI = new ProtoElementInjector(divPI, [], [], true); + var protoElementInjectors = [sectionPI, divPI, spanPI]; + + var protoWatchGroup = null; + var hasSingleRoot = false; + var pv = new ProtoView(template, diBindings, protoElementInjectors, + protoWatchGroup, hasSingleRoot); + + view = pv.instantiate(); + }); + + it('should consume text node changes', () => { + var record = new Record(null, null); + record.currentValue = 'Hello World!'; + view.onRecordChange(record , 0); + expect(view.textNodes[0].nodeValue).toEqual('Hello World!'); + }); + + it('should consume element binding changes', () => { + var elementWithBinding = view.bindElements[0]; + expect(elementWithBinding.hidden).toEqual(false); + var record = new Record(null, null); + var memento = new ElementPropertyMemento(0, 'hidden'); + record.currentValue = true; + view.onRecordChange(record, memento); + expect(elementWithBinding.hidden).toEqual(true); + }); + + it('should consume directive watch expression change.', () => { + var elInj = view.elementInjectors[1]; + + // TODO(rado): hook-up instantiateDirectives in implementation and + // remove from here. + elInj.instantiateDirectives(null); + expect(elInj.get(Directive).prop).toEqual('foo'); + var record = new Record(null, null); + var memento = new DirectivePropertyMemento(1, 0, 'prop', + (o, v) => o.prop = v); + record.currentValue = 'bar'; + view.onRecordChange(record, memento); + expect(elInj.get(Directive).prop).toEqual('bar'); + }); + }); }); }); }