fix(core): report duplicate template bindings in templates

Fixes #7315

BREAKING CHANGES:

Previously multiple template bindings on one element
(ex. `<div *ngIf='..' *ngFor='...'>`) were allowed but most of the time
were leading to undesired result. It is possible that a small number
of applications will see template parse errors that shuld be fixed by
nesting elements or using `<template>` tags explicitly.

Closes #9462
This commit is contained in:
Pawel Kozlowski 2016-06-22 17:43:27 +02:00
parent 9decc3d823
commit 098b461b69
2 changed files with 41 additions and 8 deletions

View File

@ -10,7 +10,7 @@ import {Parser} from './expression_parser/parser';
import {CompileDirectiveMetadata, CompilePipeMetadata, CompileMetadataWithType,} from './compile_metadata'; import {CompileDirectiveMetadata, CompilePipeMetadata, CompileMetadataWithType,} from './compile_metadata';
import {HtmlParser} from './html_parser'; import {HtmlParser} from './html_parser';
import {splitNsName, mergeNsAndName} from './html_tags'; import {splitNsName, mergeNsAndName} from './html_tags';
import {ParseSourceSpan, ParseError, ParseLocation, ParseErrorLevel} from './parse_util'; import {ParseSourceSpan, ParseError, ParseErrorLevel} from './parse_util';
import {InterpolationConfig} from './interpolation_config'; import {InterpolationConfig} from './interpolation_config';
import {ElementAst, BoundElementPropertyAst, BoundEventAst, ReferenceAst, TemplateAst, TemplateAstVisitor, templateVisitAll, TextAst, BoundTextAst, EmbeddedTemplateAst, AttrAst, NgContentAst, PropertyBindingType, DirectiveAst, BoundDirectivePropertyAst, ProviderAst, ProviderAstType, VariableAst} from './template_ast'; import {ElementAst, BoundElementPropertyAst, BoundEventAst, ReferenceAst, TemplateAst, TemplateAstVisitor, templateVisitAll, TextAst, BoundTextAst, EmbeddedTemplateAst, AttrAst, NgContentAst, PropertyBindingType, DirectiveAst, BoundDirectivePropertyAst, ProviderAst, ProviderAstType, VariableAst} from './template_ast';
@ -312,6 +312,19 @@ class TemplateParseVisitor implements HtmlAstVisitor {
elementOrDirectiveRefs, elementVars); elementOrDirectiveRefs, elementVars);
var hasTemplateBinding = this._parseInlineTemplateBinding( var hasTemplateBinding = this._parseInlineTemplateBinding(
attr, templateMatchableAttrs, templateElementOrDirectiveProps, templateElementVars); attr, templateMatchableAttrs, templateElementOrDirectiveProps, templateElementVars);
if (hasTemplateBinding && isTemplateElement) {
this._reportError(
`Can't have template bindings on a <template> element but the '${attr.name}' attribute was used`,
attr.sourceSpan);
}
if (hasTemplateBinding && hasInlineTemplates) {
this._reportError(
`Can't have multiple template bindings on one element. Use only one attribute named 'template' or prefixed with *`,
attr.sourceSpan);
}
if (!hasBinding && !hasTemplateBinding) { if (!hasBinding && !hasTemplateBinding) {
// don't include the bindings as attributes as well in the AST // don't include the bindings as attributes as well in the AST
attrs.push(this.visitAttr(attr, null)); attrs.push(this.visitAttr(attr, null));
@ -403,7 +416,7 @@ class TemplateParseVisitor implements HtmlAstVisitor {
private _parseInlineTemplateBinding( private _parseInlineTemplateBinding(
attr: HtmlAttrAst, targetMatchableAttrs: string[][], attr: HtmlAttrAst, targetMatchableAttrs: string[][],
targetProps: BoundElementOrDirectiveProperty[], targetVars: VariableAst[]): boolean { targetProps: BoundElementOrDirectiveProperty[], targetVars: VariableAst[]): boolean {
var templateBindingsSource: any /** TODO #???? */ = null; var templateBindingsSource: string = null;
if (attr.name == TEMPLATE_ATTR) { if (attr.name == TEMPLATE_ATTR) {
templateBindingsSource = attr.value; templateBindingsSource = attr.value;
} else if (attr.name.startsWith(TEMPLATE_ATTR_PREFIX)) { } else if (attr.name.startsWith(TEMPLATE_ATTR_PREFIX)) {
@ -812,9 +825,9 @@ class TemplateParseVisitor implements HtmlAstVisitor {
directives: DirectiveAst[], events: BoundEventAst[]) { directives: DirectiveAst[], events: BoundEventAst[]) {
var allDirectiveEvents = new Set<string>(); var allDirectiveEvents = new Set<string>();
directives.forEach(directive => { directives.forEach(directive => {
StringMapWrapper.forEach( StringMapWrapper.forEach(directive.directive.outputs, (eventName: string) => {
directive.directive.outputs, allDirectiveEvents.add(eventName);
(eventName: string, _: any /** TODO #???? */) => { allDirectiveEvents.add(eventName); }); });
}); });
events.forEach(event => { events.forEach(event => {
if (isPresent(event.target) || !SetWrapper.has(allDirectiveEvents, event.name)) { if (isPresent(event.target) || !SetWrapper.has(allDirectiveEvents, event.name)) {
@ -877,7 +890,7 @@ class ElementContext {
isTemplateElement: boolean, directives: DirectiveAst[], isTemplateElement: boolean, directives: DirectiveAst[],
providerContext: ProviderElementContext): ElementContext { providerContext: ProviderElementContext): ElementContext {
var matcher = new SelectorMatcher(); var matcher = new SelectorMatcher();
var wildcardNgContentIndex: any /** TODO #???? */ = null; var wildcardNgContentIndex: number = null;
var component = directives.find(directive => directive.directive.isComponent); var component = directives.find(directive => directive.directive.isComponent);
if (isPresent(component)) { if (isPresent(component)) {
var ngContentSelectors = component.directive.template.ngContentSelectors; var ngContentSelectors = component.directive.template.ngContentSelectors;
@ -943,7 +956,7 @@ export class PipeCollector extends RecursiveAstVisitor {
} }
function removeDuplicates(items: CompileMetadataWithType[]): CompileMetadataWithType[] { function removeDuplicates(items: CompileMetadataWithType[]): CompileMetadataWithType[] {
let res: any[] /** TODO #???? */ = []; let res: CompileMetadataWithType[] = [];
items.forEach(item => { items.forEach(item => {
let hasMatch = let hasMatch =
res.filter( res.filter(

View File

@ -26,7 +26,7 @@ var MOCK_SCHEMA_REGISTRY = [{
let zeConsole = console; let zeConsole = console;
export function main() { export function main() {
var ngIf: any /** TODO #9100 */; var ngIf: CompileDirectiveMetadata;
var parse: var parse:
(template: string, directives: CompileDirectiveMetadata[], pipes?: CompilePipeMetadata[]) => (template: string, directives: CompileDirectiveMetadata[], pipes?: CompilePipeMetadata[]) =>
TemplateAst[]; TemplateAst[];
@ -1145,6 +1145,26 @@ Reference "#a" is defined several times ("<div #a></div><div [ERROR ->]#a></div>
<ng-content> element cannot have content. <ng-content> must be immediately followed by </ng-content> ("[ERROR ->]<ng-content>content</ng-content>"): TestComp@0:0`); <ng-content> element cannot have content. <ng-content> must be immediately followed by </ng-content> ("[ERROR ->]<ng-content>content</ng-content>"): TestComp@0:0`);
}); });
it('should report when *attr is used on a template element', () => {
expect(() => parse('<template *ngIf>', [])).toThrowError(`Template parse errors:
Can't have template bindings on a <template> element but the '*ngIf' attribute was used ("<template [ERROR ->]*ngIf>"): TestComp@0:10`);
});
it('should report when a template attribute is used on a template element', () => {
expect(() => parse('<template template="ngIf">', [])).toThrowError(`Template parse errors:
Can't have template bindings on a <template> element but the 'template' attribute was used ("<template [ERROR ->]template="ngIf">"): TestComp@0:10`);
});
it('should report when mutliple *attrs are used on the same element', () => {
expect(() => parse('<div *ngIf *ngFor>', [])).toThrowError(`Template parse errors:
Can't have multiple template bindings on one element. Use only one attribute named 'template' or prefixed with * ("<div *ngIf [ERROR ->]*ngFor>"): TestComp@0:11`);
});
it('should report when mix of template and *attrs are used on the same element', () => {
expect(() => parse('<div template="ngIf" *ngFor>', [])).toThrowError(`Template parse errors:
Can't have multiple template bindings on one element. Use only one attribute named 'template' or prefixed with * ("<div template="ngIf" [ERROR ->]*ngFor>"): TestComp@0:21`);
});
it('should report invalid property names', () => { it('should report invalid property names', () => {
expect(() => parse('<div [invalidProp]></div>', [])).toThrowError(`Template parse errors: expect(() => parse('<div [invalidProp]></div>', [])).toThrowError(`Template parse errors:
Can't bind to 'invalidProp' since it isn't a known native property ("<div [ERROR ->][invalidProp]></div>"): TestComp@0:5`); Can't bind to 'invalidProp' since it isn't a known native property ("<div [ERROR ->][invalidProp]></div>"): TestComp@0:5`);