From 94e203b9df6c4b79ba30f1f08fc54ff919f722e1 Mon Sep 17 00:00:00 2001 From: Bertrand Laporte Date: Fri, 6 Feb 2015 15:41:02 -0800 Subject: [PATCH] feat(DirectiveParser): throw errors when expected directives are not present closes #527 Closes #570 --- .../angular2/src/core/compiler/compiler.js | 11 +- .../core/compiler/pipeline/compile_element.js | 46 +++++++- .../compiler/pipeline/compile_pipeline.js | 8 +- .../core/compiler/pipeline/default_steps.js | 11 +- .../compiler/pipeline/directive_parser.js | 106 +++++++++++++++--- .../pipeline/element_binder_builder.js | 17 ++- .../pipeline/property_binding_parser.js | 27 +++-- .../pipeline/text_interpolation_parser.js | 6 +- .../core/compiler/pipeline/view_splitter.js | 40 ++++++- .../angular2/src/core/compiler/selector.js | 22 +++- .../test/core/compiler/integration_spec.js | 70 +++++++++++- .../pipeline/directive_parser_spec.js | 37 +++--- .../pipeline/element_binder_builder_spec.js | 2 +- .../pipeline/property_binding_parser_spec.js | 2 +- .../text_interpolation_parser_spec.js | 2 +- .../compiler/pipeline/view_splitter_spec.js | 10 +- .../test/core/compiler/selector_spec.js | 58 +++++----- .../src/compiler/selector_benchmark.js | 2 +- 18 files changed, 354 insertions(+), 123 deletions(-) diff --git a/modules/angular2/src/core/compiler/compiler.js b/modules/angular2/src/core/compiler/compiler.js index 6b7f57800e..bfc06422ad 100644 --- a/modules/angular2/src/core/compiler/compiler.js +++ b/modules/angular2/src/core/compiler/compiler.js @@ -17,6 +17,7 @@ import {Template} from '../annotations/template'; import {ShadowDomStrategy} from './shadow_dom_strategy'; import {CompileStep} from './pipeline/compile_step'; + /** * Cache that stores the ProtoView of the template of a component. * Used to prevent duplicate work and resolve cyclic dependencies. @@ -134,7 +135,15 @@ export class Compiler { // TODO(vicb): union type return ProtoView or Promise _compileTemplate(template: Template, tplElement: Element, component: Type) { var pipeline = new CompilePipeline(this.createSteps(component, template)); - var compileElements = pipeline.process(tplElement); + var compilationCtxtDescription = stringify(this._reader.read(component).type); + var compileElements; + + try { + compileElements = pipeline.process(tplElement, compilationCtxtDescription); + } catch(ex) { + return PromiseWrapper.reject(ex); + } + var protoView = compileElements[0].inheritedProtoView; // Populate the cache before compiling the nested components, diff --git a/modules/angular2/src/core/compiler/pipeline/compile_element.js b/modules/angular2/src/core/compiler/pipeline/compile_element.js index ed0003b475..1939b192fd 100644 --- a/modules/angular2/src/core/compiler/pipeline/compile_element.js +++ b/modules/angular2/src/core/compiler/pipeline/compile_element.js @@ -1,6 +1,6 @@ import {List, Map, ListWrapper, MapWrapper} from 'angular2/src/facade/collection'; import {Element, DOM} from 'angular2/src/facade/dom'; -import {int, isBlank, isPresent, Type} from 'angular2/src/facade/lang'; +import {int, isBlank, isPresent, Type, StringJoiner, assertionsEnabled} from 'angular2/src/facade/lang'; import {DirectiveMetadata} from '../directive_metadata'; import {Decorator, Component, Viewport} from '../../annotations/annotations'; import {ElementBinder} from '../element_binder'; @@ -38,8 +38,9 @@ export class CompileElement { distanceToParentInjector:number; compileChildren: boolean; ignoreBindings: boolean; + elementDescription: string; // e.g. '
' : used to provide context in case of error - constructor(element:Element) { + constructor(element:Element, compilationUnit = '') { this.element = element; this._attrs = null; this._classList = null; @@ -66,6 +67,14 @@ export class CompileElement { this.compileChildren = true; // set to true to ignore all the bindings on the element this.ignoreBindings = false; + // description is calculated here as compilation steps may change the element + var tplDesc = assertionsEnabled()? getElementDescription(element) : null; + if (compilationUnit !== '') { + this.elementDescription = compilationUnit; + if (isPresent(tplDesc)) this.elementDescription += ": " + tplDesc; + } else { + this.elementDescription = tplDesc; + } } refreshAttrs() { @@ -165,3 +174,36 @@ export class CompileElement { return this._allDirectives; } } + +// return an HTML representation of an element start tag - without its content +// this is used to give contextual information in case of errors +function getElementDescription(domElement:Element):string { + var buf = new StringJoiner(); + var atts = DOM.attributeMap(domElement); + + buf.add("<"); + buf.add(DOM.tagName(domElement).toLowerCase()); + + // show id and class first to ease element identification + addDescriptionAttribute(buf, "id", MapWrapper.get(atts, "id")); + addDescriptionAttribute(buf, "class", MapWrapper.get(atts, "class")); + MapWrapper.forEach(atts, (attValue, attName) => { + if (attName !== "id" && attName !== "class") { + addDescriptionAttribute(buf, attName, attValue); + } + }); + + buf.add(">"); + return buf.toString(); +} + + +function addDescriptionAttribute(buffer:StringJoiner, attName:string, attValue) { + if (isPresent(attValue)) { + if (attValue.length === 0) { + buffer.add(' ' + attName); + } else { + buffer.add(' ' + attName + '="' + attValue + '"'); + } + } +} diff --git a/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js b/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js index e995be1ac7..bbe0442336 100644 --- a/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js +++ b/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js @@ -15,13 +15,13 @@ export class CompilePipeline { this._control = new CompileControl(steps); } - process(rootElement:Element):List { + process(rootElement:Element, compilationCtxtDescription:string = ''):List { var results = ListWrapper.create(); - this._process(results, null, new CompileElement(rootElement)); + this._process(results, null, new CompileElement(rootElement, compilationCtxtDescription), compilationCtxtDescription); return results; } - _process(results, parent:CompileElement, current:CompileElement) { + _process(results, parent:CompileElement, current:CompileElement, compilationCtxtDescription:string = '') { var additionalChildren = this._control.internalProcess(results, 0, parent, current); if (current.compileChildren) { @@ -31,7 +31,7 @@ export class CompilePipeline { // next sibling before recursing. var nextNode = DOM.nextSibling(node); if (DOM.isElementNode(node)) { - this._process(results, current, new CompileElement(node)); + this._process(results, current, new CompileElement(node, compilationCtxtDescription)); } node = nextNode; } diff --git a/modules/angular2/src/core/compiler/pipeline/default_steps.js b/modules/angular2/src/core/compiler/pipeline/default_steps.js index 16b180d07c..20cba05ce3 100644 --- a/modules/angular2/src/core/compiler/pipeline/default_steps.js +++ b/modules/angular2/src/core/compiler/pipeline/default_steps.js @@ -13,7 +13,6 @@ import {ShimShadowCss} from './shim_shadow_css'; import {ShimShadowDom} from './shim_shadow_dom'; import {DirectiveMetadata} from 'angular2/src/core/compiler/directive_metadata'; import {ShadowDomStrategy, EmulatedShadowDomStrategy} from 'angular2/src/core/compiler/shadow_dom_strategy'; -import {stringify} from 'angular2/src/facade/lang'; import {DOM} from 'angular2/src/facade/dom'; /** @@ -28,9 +27,7 @@ export function createDefaultSteps( directives: List, shadowDomStrategy: ShadowDomStrategy) { - var compilationUnit = stringify(compiledComponent.type); - - var steps = [new ViewSplitter(parser, compilationUnit)]; + var steps = [new ViewSplitter(parser)]; if (shadowDomStrategy instanceof EmulatedShadowDomStrategy) { var step = new ShimShadowCss(compiledComponent, shadowDomStrategy, DOM.defaultDoc().head); @@ -38,13 +35,13 @@ export function createDefaultSteps( } steps = ListWrapper.concat(steps,[ - new PropertyBindingParser(parser, compilationUnit), + new PropertyBindingParser(parser), new DirectiveParser(directives), - new TextInterpolationParser(parser, compilationUnit), + new TextInterpolationParser(parser), new ElementBindingMarker(), new ProtoViewBuilder(changeDetection, shadowDomStrategy), new ProtoElementInjectorBuilder(), - new ElementBinderBuilder(parser, compilationUnit) + new ElementBinderBuilder(parser) ]); if (shadowDomStrategy instanceof EmulatedShadowDomStrategy) { diff --git a/modules/angular2/src/core/compiler/pipeline/directive_parser.js b/modules/angular2/src/core/compiler/pipeline/directive_parser.js index 0aedf261d1..f27c9c650b 100644 --- a/modules/angular2/src/core/compiler/pipeline/directive_parser.js +++ b/modules/angular2/src/core/compiler/pipeline/directive_parser.js @@ -1,5 +1,5 @@ -import {isPresent, isBlank, BaseException} from 'angular2/src/facade/lang'; -import {List, MapWrapper} from 'angular2/src/facade/collection'; +import {isPresent, isBlank, BaseException, assertionsEnabled, RegExpWrapper} from 'angular2/src/facade/lang'; +import {List, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; import {DOM} from 'angular2/src/facade/dom'; import {SelectorMatcher} from '../selector'; import {CssSelector} from '../selector'; @@ -10,6 +10,10 @@ import {CompileStep} from './compile_step'; import {CompileElement} from './compile_element'; import {CompileControl} from './compile_control'; +import {isSpecialProperty} from './element_binder_builder';; + +var PROPERTY_BINDING_REGEXP = RegExpWrapper.create('^ *([^\\s\\|]+)'); + /** * Parses the directives on a single element. Assumes ViewSplitter has already created *