From ce6a2ba8360ba6dd83e3a053dde718a1ec4543d6 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Thu, 30 Apr 2015 14:45:42 -0700 Subject: [PATCH] refactor(view): moved the logic from ProtoView to ProtoViewFactory --- .../src/change_detection/change_detection.js | 14 +- .../src/change_detection/interfaces.js | 5 +- .../change_detection/proto_change_detector.js | 42 ++-- .../angular2/src/core/compiler/compiler.js | 15 +- .../src/core/compiler/proto_view_factory.js | 237 +++++++++++++----- modules/angular2/src/core/compiler/view.js | 130 +--------- .../src/core/compiler/view_manager_utils.js | 3 +- .../change_detection/change_detection_spec.js | 118 ++++----- .../test/core/compiler/compiler_spec.js | 11 +- .../core/compiler/element_injector_spec.js | 4 +- .../test/core/compiler/integration_spec.js | 2 + .../core/compiler/view_container_ref_spec.js | 2 +- .../test/core/compiler/view_manager_spec.js | 2 +- .../core/compiler/view_manager_utils_spec.js | 2 +- .../test/core/compiler/view_pool_spec.js | 2 +- .../change_detection_benchmark.js | 10 +- 16 files changed, 309 insertions(+), 290 deletions(-) diff --git a/modules/angular2/src/change_detection/change_detection.js b/modules/angular2/src/change_detection/change_detection.js index 70b8315c02..ae2562222b 100644 --- a/modules/angular2/src/change_detection/change_detection.js +++ b/modules/angular2/src/change_detection/change_detection.js @@ -4,9 +4,13 @@ import {IterableChangesFactory} from './pipes/iterable_changes'; import {KeyValueChangesFactory} from './pipes/keyvalue_changes'; import {AsyncPipeFactory} from './pipes/async_pipe'; import {NullPipeFactory} from './pipes/null_pipe'; +import {BindingRecord} from './binding_record'; +import {DirectiveRecord} from './directive_record'; import {DEFAULT} from './constants'; import {ChangeDetection, ProtoChangeDetector} from './interfaces'; import {Injectable} from 'angular2/di'; +import {List} from 'angular2/src/facade/collection'; + /** * Structural diffing for `Object`s and `Map`s. @@ -61,8 +65,9 @@ export class DynamicChangeDetection extends ChangeDetection { this.registry = registry; } - createProtoChangeDetector(name:string, changeControlStrategy:string = DEFAULT):ProtoChangeDetector{ - return new DynamicProtoChangeDetector(this.registry, changeControlStrategy); + createProtoChangeDetector(name:string, bindingRecords:List, variableBindings:List, + directiveRecords:List, changeControlStrategy:string = DEFAULT):ProtoChangeDetector{ + return new DynamicProtoChangeDetector(this.registry, bindingRecords, variableBindings, directiveRecords, changeControlStrategy); } } @@ -82,8 +87,9 @@ export class JitChangeDetection extends ChangeDetection { this.registry = registry; } - createProtoChangeDetector(name:string, changeControlStrategy:string = DEFAULT):ProtoChangeDetector{ - return new JitProtoChangeDetector(this.registry, changeControlStrategy); + createProtoChangeDetector(name:string, bindingRecords:List, variableBindings:List, + directiveRecords:List, changeControlStrategy:string = DEFAULT):ProtoChangeDetector{ + return new JitProtoChangeDetector(this.registry, bindingRecords, variableBindings, directiveRecords, changeControlStrategy); } } diff --git a/modules/angular2/src/change_detection/interfaces.js b/modules/angular2/src/change_detection/interfaces.js index ae83ee447b..9fe89edd38 100644 --- a/modules/angular2/src/change_detection/interfaces.js +++ b/modules/angular2/src/change_detection/interfaces.js @@ -4,7 +4,7 @@ import {DEFAULT} from './constants'; import {BindingRecord} from './binding_record'; export class ProtoChangeDetector { - instantiate(dispatcher:any, bindingRecords:List, variableBindings:List, directiveRecords:List):ChangeDetector{ + instantiate(dispatcher:any):ChangeDetector{ return null; } } @@ -33,7 +33,8 @@ export class ProtoChangeDetector { * @exportedAs angular2/change_detection */ export class ChangeDetection { - createProtoChangeDetector(name:string, changeControlStrategy:string=DEFAULT):ProtoChangeDetector{ + createProtoChangeDetector(name:string, bindingRecords:List, variableBindings:List, directiveRecords:List, + changeControlStrategy:string=DEFAULT):ProtoChangeDetector{ return null; } } diff --git a/modules/angular2/src/change_detection/proto_change_detector.js b/modules/angular2/src/change_detection/proto_change_detector.js index 4ea70c616f..f0dc58110c 100644 --- a/modules/angular2/src/change_detection/proto_change_detector.js +++ b/modules/angular2/src/change_detection/proto_change_detector.js @@ -28,7 +28,7 @@ import {DynamicChangeDetector} from './dynamic_change_detector'; import {ChangeDetectorJITGenerator} from './change_detection_jit_generator'; import {PipeRegistry} from './pipes/pipe_registry'; import {BindingRecord} from './binding_record'; -import {DirectiveIndex} from './directive_record'; +import {DirectiveRecord, DirectiveIndex} from './directive_record'; import {coalesce} from './coalesce'; @@ -50,25 +50,31 @@ import { export class DynamicProtoChangeDetector extends ProtoChangeDetector { _pipeRegistry:PipeRegistry; _records:List; + _bindingRecords:List; + _variableBindings:List; + _directiveRecords:List; _changeControlStrategy:string; - constructor(pipeRegistry:PipeRegistry, changeControlStrategy:string) { + constructor(pipeRegistry:PipeRegistry, bindingRecords:List, variableBindings:List, directiveRecords:List, changeControlStrategy:string) { super(); this._pipeRegistry = pipeRegistry; + this._bindingRecords = bindingRecords; + this._variableBindings = variableBindings; + this._directiveRecords = directiveRecords; this._changeControlStrategy = changeControlStrategy; } - instantiate(dispatcher:any, bindingRecords:List, variableBindings:List, directiveRecords:List) { - this._createRecordsIfNecessary(bindingRecords, variableBindings); + instantiate(dispatcher:any) { + this._createRecordsIfNecessary(); return new DynamicChangeDetector(this._changeControlStrategy, dispatcher, - this._pipeRegistry, this._records, directiveRecords); + this._pipeRegistry, this._records, this._directiveRecords); } - _createRecordsIfNecessary(bindingRecords:List, variableBindings:List) { + _createRecordsIfNecessary() { if (isBlank(this._records)) { var recordBuilder = new ProtoRecordBuilder(); - ListWrapper.forEach(bindingRecords, (b) => { - recordBuilder.addAst(b, variableBindings); + ListWrapper.forEach(this._bindingRecords, (b) => { + recordBuilder.addAst(b, this._variableBindings); }); this._records = coalesce(recordBuilder.records); } @@ -79,31 +85,37 @@ var _jitProtoChangeDetectorClassCounter:number = 0; export class JitProtoChangeDetector extends ProtoChangeDetector { _factory:Function; _pipeRegistry; + _bindingRecords:List; + _variableBindings:List; + _directiveRecords:List; _changeControlStrategy:string; - constructor(pipeRegistry, changeControlStrategy:string) { + constructor(pipeRegistry, bindingRecords:List, variableBindings:List, directiveRecords:List, changeControlStrategy:string) { super(); this._pipeRegistry = pipeRegistry; this._factory = null; + this._bindingRecords = bindingRecords; + this._variableBindings = variableBindings; + this._directiveRecords = directiveRecords; this._changeControlStrategy = changeControlStrategy; } - instantiate(dispatcher:any, bindingRecords:List, variableBindings:List, directiveRecords:List) { - this._createFactoryIfNecessary(bindingRecords, variableBindings, directiveRecords); + instantiate(dispatcher:any) { + this._createFactoryIfNecessary(); return this._factory(dispatcher, this._pipeRegistry); } - _createFactoryIfNecessary(bindingRecords:List, variableBindings:List, directiveRecords:List) { + _createFactoryIfNecessary() { if (isBlank(this._factory)) { var recordBuilder = new ProtoRecordBuilder(); - ListWrapper.forEach(bindingRecords, (b) => { - recordBuilder.addAst(b, variableBindings); + ListWrapper.forEach(this._bindingRecords, (b) => { + recordBuilder.addAst(b, this._variableBindings); }); var c = _jitProtoChangeDetectorClassCounter++; var records = coalesce(recordBuilder.records); var typeName = `ChangeDetector${c}`; this._factory = new ChangeDetectorJITGenerator(typeName, this._changeControlStrategy, records, - directiveRecords).generate(); + this._directiveRecords).generate(); } } } diff --git a/modules/angular2/src/core/compiler/compiler.js b/modules/angular2/src/core/compiler/compiler.js index d956e42abd..117808f42b 100644 --- a/modules/angular2/src/core/compiler/compiler.js +++ b/modules/angular2/src/core/compiler/compiler.js @@ -94,7 +94,7 @@ export class Compiler { var directiveMetadata = Compiler.buildRenderDirective(componentBinding); return this._renderer.createHostProtoView(directiveMetadata).then( (hostRenderPv) => { - return this._compileNestedProtoViews(null, hostRenderPv, [componentBinding], true); + return this._compileNestedProtoViews(null, null, hostRenderPv, [componentBinding], true); }).then( (appProtoView) => { return new ProtoViewRef(appProtoView); }); @@ -135,7 +135,7 @@ export class Compiler { if (isPresent(template.renderer)) { var directives = []; pvPromise = this._renderer.createImperativeComponentProtoView(template.renderer).then( (renderPv) => { - return this._compileNestedProtoViews(componentBinding, renderPv, directives, true); + return this._compileNestedProtoViews(null, componentBinding, renderPv, directives, true); }); } else { var directives = ListWrapper.map( @@ -144,7 +144,7 @@ export class Compiler { ); var renderTemplate = this._buildRenderTemplate(component, template, directives); pvPromise = this._renderer.compile(renderTemplate).then( (renderPv) => { - return this._compileNestedProtoViews(componentBinding, renderPv, directives, true); + return this._compileNestedProtoViews(null, componentBinding, renderPv, directives, true); }); } @@ -153,9 +153,9 @@ export class Compiler { } // TODO(tbosch): union type return AppProtoView or Promise - _compileNestedProtoViews(componentBinding, renderPv, directives, isComponentRootView) { + _compileNestedProtoViews(parentProtoView, componentBinding, renderPv, directives, isComponentRootView) { var nestedPVPromises = []; - var protoView = this._protoViewFactory.createProtoView(componentBinding, renderPv, directives); + var protoView = this._protoViewFactory.createProtoView(parentProtoView, componentBinding, renderPv, directives); if (isComponentRootView && isPresent(componentBinding)) { // Populate the cache before compiling the nested components, // so that components can reference themselves in their template. @@ -170,15 +170,12 @@ export class Compiler { var nestedRenderProtoView = renderPv.elementBinders[binderIndex].nestedProtoView; var elementBinderDone = (nestedPv) => { elementBinder.nestedProtoView = nestedPv; - // Can't set the parentProtoView for components, - // as their AppProtoView might be used in multiple other components. - nestedPv.parentProtoView = isPresent(nestedComponent) ? null : protoView; }; var nestedCall = null; if (isPresent(nestedComponent)) { nestedCall = this._compile(nestedComponent); } else if (isPresent(nestedRenderProtoView)) { - nestedCall = this._compileNestedProtoViews(componentBinding, nestedRenderProtoView, directives, false); + nestedCall = this._compileNestedProtoViews(protoView, componentBinding, nestedRenderProtoView, directives, false); } if (PromiseWrapper.isPromise(nestedCall)) { ListWrapper.push(nestedPVPromises, nestedCall.then(elementBinderDone)); diff --git a/modules/angular2/src/core/compiler/proto_view_factory.js b/modules/angular2/src/core/compiler/proto_view_factory.js index 7d44cacf54..b39560cbfe 100644 --- a/modules/angular2/src/core/compiler/proto_view_factory.js +++ b/modules/angular2/src/core/compiler/proto_view_factory.js @@ -3,13 +3,103 @@ import {List, ListWrapper, MapWrapper} from 'angular2/src/facade/collection'; import {isPresent, isBlank} from 'angular2/src/facade/lang'; import {reflector} from 'angular2/src/reflection/reflection'; -import {ChangeDetection, DirectiveIndex} from 'angular2/change_detection'; +import {ChangeDetection, DirectiveIndex, BindingRecord, DirectiveRecord, ProtoChangeDetector} from 'angular2/change_detection'; import {Component} from '../annotations_impl/annotations'; import * as renderApi from 'angular2/src/render/api'; import {AppProtoView} from './view'; import {ProtoElementInjector, DirectiveBinding} from './element_injector'; + +class BindingRecordsCreator { + _directiveRecordsMap; + _textNodeIndex:number; + + constructor() { + this._directiveRecordsMap = MapWrapper.create(); + this._textNodeIndex = 0; + } + + getBindingRecords(elementBinders:List, sortedDirectives:List):List { + var bindings = []; + + for (var boundElementIndex = 0; boundElementIndex < elementBinders.length; boundElementIndex++) { + var renderElementBinder = elementBinders[boundElementIndex]; + bindings = ListWrapper.concat(bindings, this._createTextNodeRecords(renderElementBinder)); + bindings = ListWrapper.concat(bindings, this._createElementPropertyRecords(boundElementIndex, renderElementBinder)); + bindings = ListWrapper.concat(bindings, this._createDirectiveRecords(boundElementIndex, sortedDirectives[boundElementIndex])); + } + + return bindings; + } + + getDirectiveRecords(sortedDirectives:List): List { + var directiveRecords = []; + + for (var elementIndex = 0; elementIndex < sortedDirectives.length; ++elementIndex) { + var dirs = sortedDirectives[elementIndex].directives; + for (var dirIndex = 0; dirIndex < dirs.length; ++dirIndex) { + ListWrapper.push(directiveRecords, this._getDirectiveRecord(elementIndex, dirIndex, dirs[dirIndex])); + } + } + + return directiveRecords; + } + + _createTextNodeRecords(renderElementBinder:renderApi.ElementBinder) { + if (isBlank(renderElementBinder.textBindings)) return []; + return ListWrapper.map(renderElementBinder.textBindings, b => BindingRecord.createForTextNode(b, this._textNodeIndex++)); + } + + _createElementPropertyRecords(boundElementIndex:number, renderElementBinder:renderApi.ElementBinder) { + var res = []; + MapWrapper.forEach(renderElementBinder.propertyBindings, (astWithSource, propertyName) => { + ListWrapper.push(res, BindingRecord.createForElement(astWithSource, boundElementIndex, propertyName)); + }); + return res; + } + + _createDirectiveRecords(boundElementIndex:number, sortedDirectives:SortedDirectives) { + var res = []; + for (var i = 0; i < sortedDirectives.renderDirectives.length; i++) { + var directiveBinder = sortedDirectives.renderDirectives[i]; + + // directive properties + MapWrapper.forEach(directiveBinder.propertyBindings, (astWithSource, propertyName) => { + // TODO: these setters should eventually be created by change detection, to make + // it monomorphic! + var setter = reflector.setter(propertyName); + var directiveRecord = this._getDirectiveRecord(boundElementIndex, i, sortedDirectives.directives[i]); + var b = BindingRecord.createForDirective(astWithSource, propertyName, setter, directiveRecord); + ListWrapper.push(res, b); + }); + + // host properties + MapWrapper.forEach(directiveBinder.hostPropertyBindings, (astWithSource, propertyName) => { + var dirIndex = new DirectiveIndex(boundElementIndex, i); + var b = BindingRecord.createForHostProperty(dirIndex, astWithSource, propertyName); + ListWrapper.push(res, b); + }); + } + return res; + } + + _getDirectiveRecord(boundElementIndex:number, directiveIndex:number, binding:DirectiveBinding): DirectiveRecord { + var id = boundElementIndex * 100 + directiveIndex; + + if (!MapWrapper.contains(this._directiveRecordsMap, id)) { + var changeDetection = binding.changeDetection; + + MapWrapper.set(this._directiveRecordsMap, id, + new DirectiveRecord(new DirectiveIndex(boundElementIndex, directiveIndex), + binding.callOnAllChangesDone, binding.callOnChange, changeDetection)); + } + + return MapWrapper.get(this._directiveRecordsMap, id); + } +} + + @Injectable() export class ProtoViewFactory { _changeDetection:ChangeDetection; @@ -18,40 +108,85 @@ export class ProtoViewFactory { this._changeDetection = changeDetection; } - createProtoView(componentBinding:DirectiveBinding, renderProtoView: renderApi.ProtoViewDto, - directives:List):AppProtoView { - var protoChangeDetector; - if (isBlank(componentBinding)) { - protoChangeDetector = this._changeDetection.createProtoChangeDetector('root', null); - } else { - var componentAnnotation:Component = componentBinding.annotation; - protoChangeDetector = this._changeDetection.createProtoChangeDetector( - 'dummy', componentAnnotation.changeDetection - ); - } - var protoView = new AppProtoView(renderProtoView.render, protoChangeDetector); + createProtoView(parentProtoView:AppProtoView, componentBinding:DirectiveBinding, + renderProtoView: renderApi.ProtoViewDto, directives:List):AppProtoView { + + var elementBinders = renderProtoView.elementBinders; + var sortedDirectives = ListWrapper.map(elementBinders, b => new SortedDirectives(b.directives, directives)); + + var variableBindings = this._createVariableBindings(renderProtoView); + var protoLocals = this._createProtoLocals(renderProtoView); + var variableNames = this._createVariableNames(parentProtoView, protoLocals); + var protoChangeDetector = this._createProtoChangeDetector(elementBinders, sortedDirectives, componentBinding, variableNames); + var protoView = new AppProtoView(renderProtoView.render, protoChangeDetector, variableBindings, protoLocals, variableNames); + + // TODO: vsavkin refactor to pass element binders into proto view + this._createElementBinders(protoView, elementBinders, sortedDirectives) + this._bindDirectiveEvents(protoView, sortedDirectives); - for (var i=0; i { - protoView.bindVariable(varName, mappedName); - }); return protoView; } + _createProtoLocals(renderProtoView):Map { + var protoLocals = MapWrapper.create(); + MapWrapper.forEach(renderProtoView.variableBindings, (mappedName, varName) => { + MapWrapper.set(protoLocals, mappedName, null); + }); + return protoLocals; + } + + _createVariableBindings(renderProtoView):Map { + var variableBindings = MapWrapper.create(); + MapWrapper.forEach(renderProtoView.variableBindings, (mappedName, varName) => { + MapWrapper.set(variableBindings, varName, mappedName); + }); + return variableBindings; + } + + _createVariableNames(parentProtoView, protoLocals):List { + var variableNames = isPresent(parentProtoView) ? ListWrapper.clone(parentProtoView.variableNames) : []; + MapWrapper.forEach(protoLocals, (v, local) => { + ListWrapper.push(variableNames, local); + }); + return variableNames; + } + + _createProtoChangeDetector(elementBinders, sortedDirectives, componentBinding, variableNames):ProtoChangeDetector { + var bindingRecordsCreator = new BindingRecordsCreator(); + var bindingRecords = bindingRecordsCreator.getBindingRecords(elementBinders, sortedDirectives); + var directiveRecords = bindingRecordsCreator.getDirectiveRecords(sortedDirectives); + + var changeDetection = null; + var name = 'root'; + if (isPresent(componentBinding)) { + var componentAnnotation:Component = componentBinding.annotation; + changeDetection = componentAnnotation.changeDetection; + name = 'dummy'; + } + + return this._changeDetection.createProtoChangeDetector( + name, + bindingRecords, + variableNames, + directiveRecords, + changeDetection + ); + } + + _createElementBinders(protoView, elementBinders, sortedDirectives) { + for (var i=0; i { - protoView.bindElementProperty(astWithSource, propertyName); - }); - // events - protoView.bindEvent(renderElementBinder.eventBindings, -1); + protoView.bindEvent(renderElementBinder.eventBindings, boundElementIndex, -1); // variables // The view's locals needs to have a full set of variable names at construction time // in order to prevent new variables from being set later in the lifecycle. Since we don't want @@ -127,26 +253,15 @@ export class ProtoViewFactory { return elBinder; } - _createDirectiveBinders(protoView, boundElementIndex, sortedDirectives) { - for (var i = 0; i < sortedDirectives.renderDirectives.length; i++) { - var directiveBinder = sortedDirectives.renderDirectives[i]; + _bindDirectiveEvents(protoView, sortedDirectives:List) { + for (var boundElementIndex = 0; boundElementIndex < sortedDirectives.length; ++boundElementIndex) { + var dirs = sortedDirectives[boundElementIndex].renderDirectives; + for (var i = 0; i < dirs.length; i++) { + var directiveBinder = dirs[i]; - // directive properties - MapWrapper.forEach(directiveBinder.propertyBindings, (astWithSource, propertyName) => { - // TODO: these setters should eventually be created by change detection, to make - // it monomorphic! - var setter = reflector.setter(propertyName); - protoView.bindDirectiveProperty(i, astWithSource, propertyName, setter); - }); - - // host properties - MapWrapper.forEach(directiveBinder.hostPropertyBindings, (astWithSource, propertyName) => { - var directiveIndex = new DirectiveIndex(boundElementIndex, i); - protoView.bindHostElementProperty(astWithSource, propertyName, directiveIndex); - }); - - // directive events - protoView.bindEvent(directiveBinder.eventBindings, i); + // directive events + protoView.bindEvent(directiveBinder.eventBindings, boundElementIndex, i); + } } } } diff --git a/modules/angular2/src/core/compiler/view.js b/modules/angular2/src/core/compiler/view.js index bc9e998a5f..ee7deca0d1 100644 --- a/modules/angular2/src/core/compiler/view.js +++ b/modules/angular2/src/core/compiler/view.js @@ -4,7 +4,6 @@ import {AST, Locals, ChangeDispatcher, ProtoChangeDetector, ChangeDetector, import {ProtoElementInjector, ElementInjector, PreBuiltObjects, DirectiveBinding} from './element_injector'; import {ElementBinder} from './element_binder'; -import {SetterFn} from 'angular2/src/reflection/types'; import {IMPLEMENTS, int, isPresent, isBlank, BaseException} from 'angular2/src/facade/lang'; import * as renderApi from 'angular2/src/render/api'; @@ -164,74 +163,22 @@ export class AppProtoView { protoChangeDetector:ProtoChangeDetector; variableBindings: Map; protoLocals:Map; - textNodesWithBindingCount:int; bindings:List; - parentProtoView:AppProtoView; - _variableBindings:List; - - _directiveRecordsMap:Map; - _directiveRecords:List; + variableNames:List; render:renderApi.RenderProtoViewRef; constructor( render:renderApi.RenderProtoViewRef, - protoChangeDetector:ProtoChangeDetector) { + protoChangeDetector:ProtoChangeDetector, + variableBindings:Map, + protoLocals:Map, + variableNames:List) { this.render = render; this.elementBinders = []; - this.variableBindings = MapWrapper.create(); - this.protoLocals = MapWrapper.create(); + this.variableBindings = variableBindings; + this.protoLocals = protoLocals; + this.variableNames = variableNames; this.protoChangeDetector = protoChangeDetector; - this.parentProtoView = null; - this.textNodesWithBindingCount = 0; - this.bindings = []; - this._directiveRecordsMap = MapWrapper.create(); - this._variableBindings = null; - this._directiveRecords = null; - } - - //TODO: Tobias or Victor. Moving it into the constructor. - // this work should be done the constructor of AppProtoView once we separate - // AppProtoView and ProtoViewBuilder - getVariableBindings(): List { - if (isPresent(this._variableBindings)) { - return this._variableBindings; - } - - this._variableBindings = isPresent(this.parentProtoView) ? - ListWrapper.clone(this.parentProtoView.getVariableBindings()) : []; - - MapWrapper.forEach(this.protoLocals, (v, local) => { - ListWrapper.push(this._variableBindings, local); - }); - - return this._variableBindings; - } - - //TODO: Tobias or Victor. Moving it into the constructor. - // this work should be done the constructor of ProtoView once we separate - // AppProtoView and ProtoViewBuilder - getdirectiveRecords(): List { - if (isPresent(this._directiveRecords)) { - return this._directiveRecords; - } - - this._directiveRecords = []; - - for (var injectorIndex = 0; injectorIndex < this.elementBinders.length; ++injectorIndex) { - var pei = this.elementBinders[injectorIndex].protoElementInjector; - if (isPresent(pei)) { - for (var directiveIndex = 0; directiveIndex < pei.numberOfDirectives; ++directiveIndex) { - ListWrapper.push(this._directiveRecords, this._getDirectiveRecord(injectorIndex, directiveIndex)); - } - } - } - - return this._directiveRecords; - } - - bindVariable(contextName:string, templateName:string): void { - MapWrapper.set(this.variableBindings, contextName, templateName); - MapWrapper.set(this.protoLocals, templateName, null); } bindElement(parent:ElementBinder, distanceToParent:int, protoElementInjector:ProtoElementInjector, @@ -242,32 +189,6 @@ export class AppProtoView { return elBinder; } - /** - * Adds a text node binding for the last created ElementBinder via bindElement - */ - bindTextNode(expression:AST):void { - var textNodeIndex = this.textNodesWithBindingCount++; - var b = BindingRecord.createForTextNode(expression, textNodeIndex); - ListWrapper.push(this.bindings, b); - } - - /** - * Adds an element property binding for the last created ElementBinder via bindElement - */ - bindElementProperty(expression:AST, setterName:string):void { - var elementIndex = this.elementBinders.length-1; - var b = BindingRecord.createForElement(expression, elementIndex, setterName); - ListWrapper.push(this.bindings, b); - } - - /** - * Adds an host property binding for the last created ElementBinder via bindElement - */ - bindHostElementProperty(expression:AST, setterName:string, directiveIndex:DirectiveIndex):void { - var b = BindingRecord.createForHostProperty(directiveIndex, expression, setterName); - ListWrapper.push(this.bindings, b); - } - /** * Adds an event binding for the last created ElementBinder via bindElement. * @@ -281,8 +202,8 @@ export class AppProtoView { * @param {int} directiveIndex The directive index in the binder or -1 when the event is not bound * to a directive */ - bindEvent(eventBindings: List, directiveIndex: int = -1): void { - var elBinder = this.elementBinders[this.elementBinders.length - 1]; + bindEvent(eventBindings: List, boundElementIndex:number, directiveIndex: int = -1): void { + var elBinder = this.elementBinders[boundElementIndex]; var events = elBinder.hostListeners; if (isBlank(events)) { events = StringMapWrapper.create(); @@ -299,35 +220,4 @@ export class AppProtoView { MapWrapper.set(event, directiveIndex, eventBinding.source); } } - - /** - * Adds a directive property binding for the last created ElementBinder via bindElement - */ - bindDirectiveProperty( - directiveIndex:number, - expression:AST, - setterName:string, - setter:SetterFn): void { - - var elementIndex = this.elementBinders.length-1; - var directiveRecord = this._getDirectiveRecord(elementIndex, directiveIndex); - var b = BindingRecord.createForDirective(expression, setterName, setter, directiveRecord); - ListWrapper.push(this.bindings, b); - } - - _getDirectiveRecord(elementInjectorIndex:number, directiveIndex:number): DirectiveRecord { - var id = elementInjectorIndex * 100 + directiveIndex; - var protoElementInjector = this.elementBinders[elementInjectorIndex].protoElementInjector; - - if (!MapWrapper.contains(this._directiveRecordsMap, id)) { - var binding = protoElementInjector.getDirectiveBindingAtIndex(directiveIndex); - var changeDetection = binding.changeDetection; - - MapWrapper.set(this._directiveRecordsMap, id, - new DirectiveRecord(new DirectiveIndex(elementInjectorIndex, directiveIndex), - binding.callOnAllChangesDone, binding.callOnChange, changeDetection)); - } - - return MapWrapper.get(this._directiveRecordsMap, id); - } } diff --git a/modules/angular2/src/core/compiler/view_manager_utils.js b/modules/angular2/src/core/compiler/view_manager_utils.js index acf50bc8b6..e421130b35 100644 --- a/modules/angular2/src/core/compiler/view_manager_utils.js +++ b/modules/angular2/src/core/compiler/view_manager_utils.js @@ -28,8 +28,7 @@ export class AppViewManagerUtils { createView(protoView:viewModule.AppProtoView, viewManager:avmModule.AppViewManager, renderer:Renderer): viewModule.AppView { var view = new viewModule.AppView(renderer, protoView, protoView.protoLocals); - var changeDetector = protoView.protoChangeDetector.instantiate(view, protoView.bindings, - protoView.getVariableBindings(), protoView.getdirectiveRecords()); + var changeDetector = protoView.protoChangeDetector.instantiate(view); var binders = protoView.elementBinders; var elementInjectors = ListWrapper.createFixedSize(binders.length); diff --git a/modules/angular2/test/change_detection/change_detection_spec.js b/modules/angular2/test/change_detection/change_detection_spec.js index 0bebe2c547..e666a995a2 100644 --- a/modules/angular2/test/change_detection/change_detection_spec.js +++ b/modules/angular2/test/change_detection/change_detection_spec.js @@ -16,8 +16,22 @@ import {JitProtoChangeDetector, DynamicProtoChangeDetector} from 'angular2/src/c export function main() { describe("change detection", () => { StringMapWrapper.forEach( - { "dynamic": (registry = null, strategy = null) => new DynamicProtoChangeDetector(registry, strategy), - "JIT": (registry = null, strategy = null) => new JitProtoChangeDetector(registry, strategy) + { "dynamic": (bindingRecords, variableBindings = null, directiveRecords = null, registry = null, strategy = null) => + new DynamicProtoChangeDetector( + registry, + isBlank(bindingRecords) ? [] : bindingRecords, + isBlank(variableBindings) ? [] : variableBindings, + isBlank(directiveRecords) ? [] : directiveRecords, + strategy), + + "JIT": (bindingRecords, variableBindings = null, directiveRecords = null, registry = null, strategy = null) => + new JitProtoChangeDetector( + registry, + isBlank(bindingRecords) ? [] : bindingRecords, + isBlank(variableBindings) ? [] : variableBindings, + isBlank(directiveRecords) ? [] : directiveRecords, + strategy) + }, (createProtoChangeDetector, name) => { if (name == "JIT" && IS_DARTIUM) return; @@ -43,13 +57,13 @@ export function main() { } function createChangeDetector(propName:string, exp:string, context = null, locals = null, registry = null) { - var pcd = createProtoChangeDetector(registry); var dispatcher = new TestDispatcher(); var variableBindings = convertLocalsToVariableBindings(locals); var records = [BindingRecord.createForElement(ast(exp), 0, propName)]; - var cd = pcd.instantiate(dispatcher, records, variableBindings, []); + var pcd = createProtoChangeDetector(records, variableBindings, [], registry); + var cd = pcd.instantiate(dispatcher); cd.hydrate(context, locals, null); return {"changeDetector" : cd, "dispatcher" : dispatcher}; @@ -61,11 +75,6 @@ export function main() { return res["dispatcher"].log; } - function instantiate(protoChangeDetector, dispatcher, bindings, directiveRecords = null) { - if (isBlank(directiveRecords)) directiveRecords = []; - return protoChangeDetector.instantiate(dispatcher, bindings, null, directiveRecords); - } - describe(`${name} change detection`, () => { var dispatcher; @@ -205,10 +214,10 @@ export function main() { }); it("should support interpolation", () => { - var pcd = createProtoChangeDetector(); var ast = parser.parseInterpolation("B{{a}}A", "location"); + var pcd = createProtoChangeDetector([BindingRecord.createForElement(ast, 0, "memo")]); - var cd = instantiate(pcd, dispatcher, [BindingRecord.createForElement(ast, 0, "memo")]); + var cd = pcd.instantiate(dispatcher); cd.hydrate(new TestData("value"), null, null); cd.detectChanges(); @@ -267,10 +276,9 @@ export function main() { }); it("should happen directly, without invoking the dispatcher", () => { - var pcd = createProtoChangeDetector(); + var pcd = createProtoChangeDetector([updateA("42", dirRecord1)], [], [dirRecord1]); - var cd = instantiate(pcd, dispatcher, [updateA("42", dirRecord1)], - [dirRecord1]); + var cd = pcd.instantiate(dispatcher); cd.hydrate(null, null, dirs([directive1])); @@ -282,13 +290,13 @@ export function main() { describe("onChange", () => { it("should notify the directive when a group of records changes", () => { - var pcd = createProtoChangeDetector(); - - var cd = instantiate(pcd, dispatcher, [ + var pcd = createProtoChangeDetector([ updateA("1", dirRecord1), updateB("2", dirRecord1), updateA("3", dirRecord2) - ], [dirRecord1, dirRecord2]); + ], [], [dirRecord1, dirRecord2]); + + var cd = pcd.instantiate(dispatcher); cd.hydrate(null, null, dirs([directive1, directive2])); @@ -299,11 +307,11 @@ export function main() { }); it("should not call onChange when callOnChange is false", () => { - var pcd = createProtoChangeDetector(); - - var cd = instantiate(pcd, dispatcher, [ + var pcd = createProtoChangeDetector([ updateA("1", dirRecordNoCallbacks) - ], [dirRecordNoCallbacks]); + ], [], [dirRecordNoCallbacks]); + + var cd = pcd.instantiate(dispatcher); cd.hydrate(null, null, dirs([directive1])); @@ -315,9 +323,9 @@ export function main() { describe("onAllChangesDone", () => { it("should be called after processing all the children", () => { - var pcd = createProtoChangeDetector(); + var pcd = createProtoChangeDetector([], [], [dirRecord1, dirRecord2]); - var cd = instantiate(pcd, dispatcher, [], [dirRecord1, dirRecord2]); + var cd = pcd.instantiate(dispatcher); cd.hydrate(null, null, dirs([directive1, directive2])); cd.detectChanges(); @@ -328,11 +336,11 @@ export function main() { it("should not be called when onAllChangesDone is false", () => { - var pcd = createProtoChangeDetector(); - - var cd = instantiate(pcd, dispatcher, [ + var pcd = createProtoChangeDetector([ updateA("1", dirRecordNoCallbacks) - ], [dirRecordNoCallbacks]); + ], [], [dirRecordNoCallbacks]); + + var cd = pcd.instantiate(dispatcher); cd.hydrate(null, null, dirs([directive1])); @@ -342,8 +350,8 @@ export function main() { }); it("should be called in reverse order so the child is always notified before the parent", () => { - var pcd = createProtoChangeDetector(); - var cd = instantiate(pcd, dispatcher, [], [dirRecord1, dirRecord2]); + var pcd = createProtoChangeDetector([], [], [dirRecord1, dirRecord2]); + var cd = pcd.instantiate(dispatcher); var onChangesDoneCalls = []; var td1; @@ -358,13 +366,12 @@ export function main() { }); it("should be called before processing shadow dom children", () => { - var pcd = createProtoChangeDetector(); - var shadowDomChildPCD = createProtoChangeDetector(); + var pcd = createProtoChangeDetector([], null, [dirRecord1]); + var shadowDomChildPCD = createProtoChangeDetector([updateA("1", dirRecord1)], null, [dirRecord1]); - var parent = pcd.instantiate(dispatcher, [], null, [dirRecord1]); + var parent = pcd.instantiate(dispatcher); - var child = shadowDomChildPCD.instantiate(dispatcher, - [updateA("1", dirRecord1)], null, [dirRecord1]); + var child = shadowDomChildPCD.instantiate(dispatcher); parent.addShadowDomChild(child); var directiveInShadowDom = new TestDirective(); @@ -389,8 +396,8 @@ export function main() { var directive = new TestDirective(); directive.a = "aaa"; - var pcd = createProtoChangeDetector(); - var cd = instantiate(pcd, dispatcher, [BindingRecord.createForHostProperty(index, ast("a"), "prop")], [dirRecord]); + var pcd = createProtoChangeDetector([BindingRecord.createForHostProperty(index, ast("a"), "prop")], null, [dirRecord]); + var cd = pcd.instantiate(dispatcher); cd.hydrate(null, null, dirs([directive])); cd.detectChanges(); @@ -401,12 +408,12 @@ export function main() { describe("enforce no new changes", () => { it("should throw when a record gets changed after it has been checked", () => { - var pcd = createProtoChangeDetector(); - - var dispatcher = new TestDispatcher(); - var cd = instantiate(pcd, dispatcher, [ + var pcd = createProtoChangeDetector([ BindingRecord.createForElement(ast("a"), 0, "a") ]); + + var dispatcher = new TestDispatcher(); + var cd = pcd.instantiate(dispatcher); cd.hydrate(new TestData('value'), null, null); expect(() => { @@ -487,11 +494,8 @@ export function main() { var parent, child; beforeEach(() => { - var protoParent = createProtoChangeDetector(); - parent = instantiate(protoParent, null, []); - - var protoChild = createProtoChangeDetector(); - child = instantiate(protoChild, null, []); + parent = createProtoChangeDetector([]).instantiate(null); + child = createProtoChangeDetector([]).instantiate(null); }); it("should add light dom children", () => { @@ -526,8 +530,8 @@ export function main() { describe("mode", () => { it("should set the mode to CHECK_ALWAYS when the default change detection is used", () => { - var proto = createProtoChangeDetector(null, DEFAULT); - var cd = proto.instantiate(null, [], [], []); + var proto = createProtoChangeDetector([], [], [], null, DEFAULT); + var cd = proto.instantiate(null); expect(cd.mode).toEqual(null); @@ -537,8 +541,8 @@ export function main() { }); it("should set the mode to CHECK_ONCE when the push change detection is used", () => { - var proto = createProtoChangeDetector(null, ON_PUSH); - var cd = proto.instantiate(null, [], [], []); + var proto = createProtoChangeDetector([], [], [], null, ON_PUSH); + var cd = proto.instantiate(null); cd.hydrate(null, null, null); expect(cd.mode).toEqual(CHECK_ONCE); @@ -567,7 +571,7 @@ export function main() { }); it("should change CHECK_ONCE to CHECKED", () => { - var cd = instantiate(createProtoChangeDetector(), null, []); + var cd = createProtoChangeDetector([]).instantiate(null); cd.mode = CHECK_ONCE; cd.detectChanges(); @@ -576,7 +580,7 @@ export function main() { }); it("should not change the CHECK_ALWAYS", () => { - var cd = instantiate(createProtoChangeDetector(), null, []); + var cd = createProtoChangeDetector([]).instantiate(null); cd.mode = CHECK_ALWAYS; cd.detectChanges(); @@ -591,8 +595,8 @@ export function main() { var directives; beforeEach(() => { - var proto = createProtoChangeDetector(null, ON_PUSH); - checkedDetector = instantiate(proto, null, [], []); + var proto = createProtoChangeDetector([], [], [], null, ON_PUSH); + checkedDetector = proto.instantiate(null); checkedDetector.hydrate(null, null, null); checkedDetector.mode = CHECKED; @@ -608,9 +612,9 @@ export function main() { }); it("should set the mode to CHECK_ONCE when a binding is updated", () => { - var proto = createProtoChangeDetector(null); + var proto = createProtoChangeDetector([updateDirWithOnPushRecord], [], [dirRecordWithOnPush]); - var cd = instantiate(proto, null, [updateDirWithOnPushRecord], [dirRecordWithOnPush]); + var cd = proto.instantiate(null); cd.hydrate(null, null, directives); expect(checkedDetector.mode).toEqual(CHECKED); @@ -625,7 +629,7 @@ export function main() { describe("markPathToRootAsCheckOnce", () => { function changeDetector(mode, parent) { - var cd = instantiate(createProtoChangeDetector(), null, []); + var cd = createProtoChangeDetector([]).instantiate(null); cd.mode = mode; if (isPresent(parent)) parent.addChild(cd); return cd; diff --git a/modules/angular2/test/core/compiler/compiler_spec.js b/modules/angular2/test/core/compiler/compiler_spec.js index f55b8e9eb2..33915058bb 100644 --- a/modules/angular2/test/core/compiler/compiler_spec.js +++ b/modules/angular2/test/core/compiler/compiler_spec.js @@ -283,9 +283,6 @@ export function main() { compiler.compile(MainComponent).then( (protoViewRef) => { expect(internalProtoView(protoViewRef)).toBe(mainProtoView); expect(mainProtoView.elementBinders[0].nestedProtoView).toBe(nestedProtoView); - // parentProtoView of nested components has to be null as components can - // be used by multiple other components. - expect(nestedProtoView.parentProtoView).toBe(null); async.done(); }); })); @@ -316,11 +313,7 @@ export function main() { compiler.compile(MainComponent).then( (protoViewRef) => { expect(internalProtoView(protoViewRef)).toBe(mainProtoView); expect(mainProtoView.elementBinders[0].nestedProtoView).toBe(viewportProtoView); - expect(viewportProtoView.parentProtoView).toBe(mainProtoView); expect(viewportProtoView.elementBinders[0].nestedProtoView).toBe(nestedProtoView); - // parentProtoView of nested components has to be null as components can - // be used by multiple other components. - expect(nestedProtoView.parentProtoView).toBe(null); async.done(); }); @@ -432,7 +425,7 @@ function createDirectiveBinding(reader, type) { } function createProtoView(elementBinders = null) { - var pv = new AppProtoView(null, null); + var pv = new AppProtoView(null, null, null, null, null); if (isBlank(elementBinders)) { elementBinders = []; } @@ -575,7 +568,7 @@ class FakeProtoViewFactory extends ProtoViewFactory { this._results = results; } - createProtoView(componentBinding:DirectiveBinding, renderProtoView: renderApi.ProtoViewDto, + createProtoView(parentProtoView, componentBinding:DirectiveBinding, renderProtoView: renderApi.ProtoViewDto, directives:List):AppProtoView { ListWrapper.push(this.requests, [componentBinding, renderProtoView, directives]); return ListWrapper.removeAt(this._results, 0); diff --git a/modules/angular2/test/core/compiler/element_injector_spec.js b/modules/angular2/test/core/compiler/element_injector_spec.js index 78963cb7ba..259ebb4d14 100644 --- a/modules/angular2/test/core/compiler/element_injector_spec.js +++ b/modules/angular2/test/core/compiler/element_injector_spec.js @@ -453,7 +453,7 @@ export function main() { }); it("should instantiate directives that depend on pre built objects", function () { - var protoView = new AppProtoView(null, null); + var protoView = new AppProtoView(null, null, null, null, null); var inj = injector([NeedsProtoViewRef], null, null, new PreBuiltObjects(null, null, protoView)); expect(inj.get(NeedsProtoViewRef).protoViewRef).toEqual(new ProtoViewRef(protoView)); @@ -700,7 +700,7 @@ export function main() { }); it("should inject ProtoViewRef", function () { - var protoView = new AppProtoView(null, null); + var protoView = new AppProtoView(null, null, null, null, null); var inj = injector([NeedsProtoViewRef], null, null, new PreBuiltObjects(null, null, protoView)); expect(inj.get(NeedsProtoViewRef).protoViewRef).toEqual(new ProtoViewRef(protoView)); diff --git a/modules/angular2/test/core/compiler/integration_spec.js b/modules/angular2/test/core/compiler/integration_spec.js index 64fe185588..7312b77b92 100644 --- a/modules/angular2/test/core/compiler/integration_spec.js +++ b/modules/angular2/test/core/compiler/integration_spec.js @@ -14,6 +14,7 @@ import { xit } from 'angular2/test_lib'; + import {TestBed} from 'angular2/src/test_lib/test_bed'; import {DOM} from 'angular2/src/dom/dom_adapter'; @@ -46,6 +47,7 @@ export function main() { ctx = new MyComp(); }); + describe('react to record changes', function() { it('should consume text node changes', inject([TestBed, AsyncTestCompleter], (tb, async) => { tb.overrideView(MyComp, new View({template: '
{{ctxProp}}
'})); diff --git a/modules/angular2/test/core/compiler/view_container_ref_spec.js b/modules/angular2/test/core/compiler/view_container_ref_spec.js index 050677a2fd..7412e2df29 100644 --- a/modules/angular2/test/core/compiler/view_container_ref_spec.js +++ b/modules/angular2/test/core/compiler/view_container_ref_spec.js @@ -37,7 +37,7 @@ export function main() { } function createProtoView() { - return new AppProtoView(null, null); + return new AppProtoView(null, null, null, null, null); } function createView() { diff --git a/modules/angular2/test/core/compiler/view_manager_spec.js b/modules/angular2/test/core/compiler/view_manager_spec.js index a647273916..014509c1f4 100644 --- a/modules/angular2/test/core/compiler/view_manager_spec.js +++ b/modules/angular2/test/core/compiler/view_manager_spec.js @@ -80,7 +80,7 @@ export function main() { staticChildComponentCount++; } } - var res = new AppProtoView(new MockProtoViewRef(staticChildComponentCount), null); + var res = new AppProtoView(new MockProtoViewRef(staticChildComponentCount), null, null, null, null); res.elementBinders = binders; return res; } diff --git a/modules/angular2/test/core/compiler/view_manager_utils_spec.js b/modules/angular2/test/core/compiler/view_manager_utils_spec.js index 8b6996990a..fd492aec59 100644 --- a/modules/angular2/test/core/compiler/view_manager_utils_spec.js +++ b/modules/angular2/test/core/compiler/view_manager_utils_spec.js @@ -60,7 +60,7 @@ export function main() { if (isBlank(binders)) { binders = []; } - var res = new AppProtoView(null, null); + var res = new AppProtoView(null, null, null, null, null); res.elementBinders = binders; return res; } diff --git a/modules/angular2/test/core/compiler/view_pool_spec.js b/modules/angular2/test/core/compiler/view_pool_spec.js index 55c848eb3c..eb6af39865 100644 --- a/modules/angular2/test/core/compiler/view_pool_spec.js +++ b/modules/angular2/test/core/compiler/view_pool_spec.js @@ -26,7 +26,7 @@ export function main() { } function createProtoView() { - return new AppProtoView(null, null); + return new AppProtoView(null, null, null, null, null); } function createView(pv) { diff --git a/modules/benchmarks/src/change_detection/change_detection_benchmark.js b/modules/benchmarks/src/change_detection/change_detection_benchmark.js index 7800bc45b3..4c3863a2f2 100644 --- a/modules/benchmarks/src/change_detection/change_detection_benchmark.js +++ b/modules/benchmarks/src/change_detection/change_detection_benchmark.js @@ -187,10 +187,8 @@ function setUpChangeDetection(changeDetection:ChangeDetection, iterations, objec var dispatcher = new DummyDispatcher(); var parser = new Parser(new Lexer()); - var parentProto = changeDetection.createProtoChangeDetector('parent'); - var parentCd = parentProto.instantiate(dispatcher, [], [], []); - - var proto = changeDetection.createProtoChangeDetector("proto"); + var parentProto = changeDetection.createProtoChangeDetector('parent', [], [], []); + var parentCd = parentProto.instantiate(dispatcher); var directiveRecord = new DirectiveRecord(new DirectiveIndex(0, 0), false, false, DEFAULT); var bindings = [ @@ -206,9 +204,11 @@ function setUpChangeDetection(changeDetection:ChangeDetection, iterations, objec BindingRecord.createForDirective(parser.parseBinding('field9', null), "field9", reflector.setter("field9"), directiveRecord) ]; + var proto = changeDetection.createProtoChangeDetector("proto", bindings, [], [directiveRecord]); + var targetObj = new Obj(); for (var i = 0; i < iterations; ++i) { - var cd = proto.instantiate(dispatcher, bindings, [], [directiveRecord]); + var cd = proto.instantiate(dispatcher); cd.hydrate(object, null, new FakeDirectives(targetObj)); parentCd.addChild(cd); }