From b4338b623cbfa2d1321f137ba74b1cc2f1e260bd Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 9 Jan 2015 18:48:28 +0100 Subject: [PATCH] fix(compiler): fix directive registration order fix #328 --- .../core/src/compiler/directive_metadata.js | 2 +- .../src/compiler/pipeline/compile_element.js | 24 +++++++++++- .../pipeline/element_binder_builder.js | 39 ++++++------------- .../proto_element_injector_builder.js | 19 +-------- .../core/test/compiler/integration_spec.js | 18 +++++++++ .../pipeline/directive_parser_spec.js | 8 ++++ .../pipeline/element_binder_builder_spec.js | 7 ++-- .../proto_element_injector_builder_spec.js | 6 ++- 8 files changed, 71 insertions(+), 52 deletions(-) diff --git a/modules/core/src/compiler/directive_metadata.js b/modules/core/src/compiler/directive_metadata.js index 056f6c3eb0..0f868d8f97 100644 --- a/modules/core/src/compiler/directive_metadata.js +++ b/modules/core/src/compiler/directive_metadata.js @@ -1,4 +1,4 @@ -import {Type, FIELD} from 'facade/lang'; +import {Type} from 'facade/lang'; import {Directive} from '../annotations/annotations' import {List} from 'facade/collection' import {ShadowDomStrategy} from './shadow_dom'; diff --git a/modules/core/src/compiler/pipeline/compile_element.js b/modules/core/src/compiler/pipeline/compile_element.js index bbe8f79ab4..ceb05425f3 100644 --- a/modules/core/src/compiler/pipeline/compile_element.js +++ b/modules/core/src/compiler/pipeline/compile_element.js @@ -1,6 +1,6 @@ import {List, Map, ListWrapper, MapWrapper} from 'facade/collection'; import {Element, DOM} from 'facade/dom'; -import {int, isBlank, isPresent} from 'facade/lang'; +import {int, isBlank, isPresent, Type} from 'facade/lang'; import {DirectiveMetadata} from '../directive_metadata'; import {Decorator} from '../../annotations/annotations'; import {Component} from '../../annotations/annotations'; @@ -27,6 +27,7 @@ export class CompileElement { decoratorDirectives:List; templateDirective:DirectiveMetadata; componentDirective:DirectiveMetadata; + _allDirectives:List; isViewRoot:boolean; hasBindings:boolean; inheritedProtoView:ProtoView; @@ -45,6 +46,7 @@ export class CompileElement { this.decoratorDirectives = null; this.templateDirective = null; this.componentDirective = null; + this._allDirectives = null; this.isViewRoot = false; this.hasBindings = false; // inherited down to children if they don't have @@ -116,6 +118,7 @@ export class CompileElement { addDirective(directive:DirectiveMetadata) { var annotation = directive.annotation; + this._allDirectives = null; if (annotation instanceof Decorator) { if (isBlank(this.decoratorDirectives)) { this.decoratorDirectives = ListWrapper.create(); @@ -130,4 +133,23 @@ export class CompileElement { this.componentDirective = directive; } } + + getAllDirectives(): List { + if (this._allDirectives === null) { + // Collect all the directives + // When present the component directive must be first + var directives = ListWrapper.create(); + if (isPresent(this.componentDirective)) { + ListWrapper.push(directives, this.componentDirective); + } + if (isPresent(this.templateDirective)) { + ListWrapper.push(directives, this.templateDirective); + } + if (isPresent(this.decoratorDirectives)) { + directives = ListWrapper.concat(directives, this.decoratorDirectives); + } + this._allDirectives = directives; + } + return this._allDirectives; + } } diff --git a/modules/core/src/compiler/pipeline/element_binder_builder.js b/modules/core/src/compiler/pipeline/element_binder_builder.js index 95967bdaf4..ddd29d4533 100644 --- a/modules/core/src/compiler/pipeline/element_binder_builder.js +++ b/modules/core/src/compiler/pipeline/element_binder_builder.js @@ -58,7 +58,7 @@ export class ElementBinderBuilder extends CompileStep { if (isPresent(current.eventBindings)) { this._bindEvents(protoView, current); } - this._bindDirectiveProperties(this._collectDirectives(current), current); + this._bindDirectiveProperties(current.getAllDirectives(), current); } else if (isPresent(parent)) { elementBinder = parent.inheritedElementBinder; } @@ -85,37 +85,21 @@ export class ElementBinderBuilder extends CompileStep { }); } - _collectDirectives(compileElement) { - var directives; - if (isPresent(compileElement.decoratorDirectives)) { - directives = ListWrapper.clone(compileElement.decoratorDirectives); - } else { - directives = []; - } - if (isPresent(compileElement.templateDirective)) { - ListWrapper.push(directives, compileElement.templateDirective); - } - if (isPresent(compileElement.componentDirective)) { - ListWrapper.push(directives, compileElement.componentDirective); - } - return directives; - } - - _bindDirectiveProperties(typesWithAnnotations, compileElement) { + _bindDirectiveProperties(directives: List, + compileElement: CompileElement) { var protoView = compileElement.inheritedProtoView; - var directiveIndex = 0; - ListWrapper.forEach(typesWithAnnotations, (typeWithAnnotation) => { - var annotation = typeWithAnnotation.annotation; - if (isBlank(annotation.bind)) { - return; - } - StringMapWrapper.forEach(annotation.bind, (dirProp, elProp) => { + + for (var directiveIndex = 0; directiveIndex < directives.length; directiveIndex++) { + var directive = ListWrapper.get(directives, directiveIndex); + var annotation = directive.annotation; + if (isBlank(annotation.bind)) continue; + StringMapWrapper.forEach(annotation.bind, function (dirProp, elProp) { var expression = isPresent(compileElement.propertyBindings) ? MapWrapper.get(compileElement.propertyBindings, elProp) : null; if (isBlank(expression)) { throw new BaseException('No element binding found for property '+elProp - +' which is required by directive '+stringify(typeWithAnnotation.type)); + +' which is required by directive '+stringify(directive.type)); } var len = dirProp.length; var dirBindingName = dirProp; @@ -129,7 +113,6 @@ export class ElementBinderBuilder extends CompileStep { isContentWatch ); }); - directiveIndex++; - }); + } } } diff --git a/modules/core/src/compiler/pipeline/proto_element_injector_builder.js b/modules/core/src/compiler/pipeline/proto_element_injector_builder.js index d485e57887..8b27bfb998 100644 --- a/modules/core/src/compiler/pipeline/proto_element_injector_builder.js +++ b/modules/core/src/compiler/pipeline/proto_element_injector_builder.js @@ -32,8 +32,7 @@ export class ProtoElementInjectorBuilder extends CompileStep { process(parent:CompileElement, current:CompileElement, control:CompileControl) { var distanceToParentInjector = this._getDistanceToParentInjector(parent, current); var parentProtoElementInjector = this._getParentProtoElementInjector(parent, current); - var injectorBindings = this._collectDirectiveBindings(current); - + var injectorBindings = ListWrapper.map(current.getAllDirectives(), this._createBinding); // TODO: add lightDomServices as well, // but after the directives as we rely on that order // in the element_binder_builder. @@ -65,22 +64,6 @@ export class ProtoElementInjectorBuilder extends CompileStep { return null; } - _collectDirectiveBindings(pipelineElement) { - var directiveTypes = []; - if (isPresent(pipelineElement.componentDirective)) { - ListWrapper.push(directiveTypes, this._createBinding(pipelineElement.componentDirective)); - } - if (isPresent(pipelineElement.templateDirective)) { - ListWrapper.push(directiveTypes, this._createBinding(pipelineElement.templateDirective)); - } - if (isPresent(pipelineElement.decoratorDirectives)) { - for (var i=0; i { + compiler.compile(MyComp, el('')).then((pv) => { + createView(pv); + + ctx.ctxProp = 'Hello World!'; + cd.detectChanges(); + + var elInj = view.elementInjectors[0]; + expect(elInj.get(MyDir).dirProp).toEqual('Hello World!'); + expect(elInj.get(ChildComp).dirProp).toEqual(null); + + done(); + }); + }); + it('should support template directives via `