From 881eb894bc17092e61842e8b37955344829510c9 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 27 Dec 2016 15:23:49 -0800 Subject: [PATCH] fix(Compiler): allow "." in attribute selectors (#13653) fixes #13645 --- modules/@angular/compiler/src/selector.ts | 12 +- .../src/template_parser/template_parser.ts | 36 +++-- .../compiler/test/integration_spec.ts | 51 +++++++ .../@angular/compiler/test/selector_spec.ts | 131 ++++++++++++------ .../template_parser/template_parser_spec.ts | 28 +++- modules/@angular/upgrade/src/util.ts | 4 +- 6 files changed, 193 insertions(+), 69 deletions(-) create mode 100644 modules/@angular/compiler/test/integration_spec.ts diff --git a/modules/@angular/compiler/src/selector.ts b/modules/@angular/compiler/src/selector.ts index 2934414ac3..a34ba9af28 100644 --- a/modules/@angular/compiler/src/selector.ts +++ b/modules/@angular/compiler/src/selector.ts @@ -9,12 +9,12 @@ import {getHtmlTagDefinition} from './ml_parser/html_tags'; const _SELECTOR_REGEXP = new RegExp( - '(\\:not\\()|' + //":not(" - '([-\\w]+)|' + // "tag" - '(?:\\.([-\\w]+))|' + // ".class" - '(?:\\[([-\\w*]+)(?:=([^\\]]*))?\\])|' + // "[name]", "[name=value]" - '(\\))|' + // ")" - '(\\s*,\\s*)', // "," + '(\\:not\\()|' + //":not(" + '([-\\w]+)|' + // "tag" + '(?:\\.([-\\w]+))|' + // ".class" + '(?:\\[([.-\\w*]+)(?:=([^\\]]*))?\\])|' + // "[name]", "[name=value]" + '(\\))|' + // ")" + '(\\s*,\\s*)', // "," 'g'); /** diff --git a/modules/@angular/compiler/src/template_parser/template_parser.ts b/modules/@angular/compiler/src/template_parser/template_parser.ts index 770727f1cf..c5e0c08f32 100644 --- a/modules/@angular/compiler/src/template_parser/template_parser.ts +++ b/modules/@angular/compiler/src/template_parser/template_parser.ts @@ -149,7 +149,7 @@ export class TemplateParser { return new TemplateParseResult(result, errors); } - if (isPresent(this.transforms)) { + if (this.transforms) { this.transforms.forEach( (transform: TemplateAstVisitor) => { result = templateVisitAll(transform, result); }); } @@ -218,7 +218,7 @@ class TemplateParseVisitor implements html.Visitor { visitText(text: html.Text, parent: ElementContext): any { const ngContentIndex = parent.findNgContentIndex(TEXT_CSS_SELECTOR); const expr = this._bindingParser.parseInterpolation(text.value, text.sourceSpan); - if (isPresent(expr)) { + if (expr) { return new BoundTextAst(expr, ngContentIndex, text.sourceSpan); } else { return new TextAst(text.value, ngContentIndex, text.sourceSpan); @@ -248,14 +248,14 @@ class TemplateParseVisitor implements html.Visitor { return null; } - const matchableAttrs: string[][] = []; + const matchableAttrs: [string, string][] = []; const elementOrDirectiveProps: BoundProperty[] = []; const elementOrDirectiveRefs: ElementOrDirectiveRef[] = []; const elementVars: VariableAst[] = []; const events: BoundEventAst[] = []; const templateElementOrDirectiveProps: BoundProperty[] = []; - const templateMatchableAttrs: string[][] = []; + const templateMatchableAttrs: [string, string][] = []; const templateElementVars: VariableAst[] = []; let hasInlineTemplates = false; @@ -268,14 +268,17 @@ class TemplateParseVisitor implements html.Visitor { isTemplateElement, attr, matchableAttrs, elementOrDirectiveProps, events, elementOrDirectiveRefs, elementVars); - let templateBindingsSource: string|undefined = undefined; - let prefixToken: string|undefined = undefined; - if (this._normalizeAttributeName(attr.name) == TEMPLATE_ATTR) { + let templateBindingsSource: string|undefined; + let prefixToken: string|undefined; + let normalizedName = this._normalizeAttributeName(attr.name); + + if (normalizedName == TEMPLATE_ATTR) { templateBindingsSource = attr.value; - } else if (attr.name.startsWith(TEMPLATE_ATTR_PREFIX)) { + } else if (normalizedName.startsWith(TEMPLATE_ATTR_PREFIX)) { templateBindingsSource = attr.value; - prefixToken = attr.name.substring(TEMPLATE_ATTR_PREFIX.length); // remove the star + prefixToken = normalizedName.substring(TEMPLATE_ATTR_PREFIX.length); } + const hasTemplateBinding = isPresent(templateBindingsSource); if (hasTemplateBinding) { if (hasInlineTemplates) { @@ -541,10 +544,12 @@ class TemplateParseVisitor implements html.Visitor { elementSourceSpan: ParseSourceSpan, targetReferences: ReferenceAst[]): DirectiveAst[] { const matchedReferences = new Set(); let component: CompileDirectiveSummary = null; + const directiveAsts = directives.map((directive) => { const sourceSpan = new ParseSourceSpan( elementSourceSpan.start, elementSourceSpan.end, `Directive ${identifierName(directive.type)}`); + if (directive.isComponent) { component = directive; } @@ -567,6 +572,7 @@ class TemplateParseVisitor implements html.Visitor { return new DirectiveAst( directive, directiveProperties, hostProperties, hostEvents, sourceSpan); }); + elementOrDirectiveRefs.forEach((elOrDirRef) => { if (elOrDirRef.value.length > 0) { if (!matchedReferences.has(elOrDirRef.name)) { @@ -581,7 +587,7 @@ class TemplateParseVisitor implements html.Visitor { } targetReferences.push(new ReferenceAst(elOrDirRef.name, refToken, elOrDirRef.sourceSpan)); } - }); // fix syntax highlighting issue: ` + }); return directiveAsts; } @@ -742,7 +748,7 @@ class NonBindableVisitor implements html.Visitor { return null; } - const attrNameAndValues = ast.attrs.map(attrAst => [attrAst.name, attrAst.value]); + const attrNameAndValues = ast.attrs.map((attr): [string, string] => [attr.name, attr.value]); const selector = createElementCssSelector(ast.name, attrNameAndValues); const ngContentIndex = parent.findNgContentIndex(selector); const children = html.visitAll(this, ast.children, EMPTY_ELEMENT_CONTEXT); @@ -811,16 +817,16 @@ class ElementContext { } export function createElementCssSelector( - elementName: string, matchableAttrs: string[][]): CssSelector { + elementName: string, attributes: [string, string][]): CssSelector { const cssSelector = new CssSelector(); const elNameNoNs = splitNsName(elementName)[1]; cssSelector.setElement(elNameNoNs); - for (let i = 0; i < matchableAttrs.length; i++) { - const attrName = matchableAttrs[i][0]; + for (let i = 0; i < attributes.length; i++) { + const attrName = attributes[i][0]; const attrNameNoNs = splitNsName(attrName)[1]; - const attrValue = matchableAttrs[i][1]; + const attrValue = attributes[i][1]; cssSelector.addAttribute(attrNameNoNs, attrValue); if (attrName.toLowerCase() == CLASS_ATTR) { diff --git a/modules/@angular/compiler/test/integration_spec.ts b/modules/@angular/compiler/test/integration_spec.ts new file mode 100644 index 0000000000..95051bc688 --- /dev/null +++ b/modules/@angular/compiler/test/integration_spec.ts @@ -0,0 +1,51 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {Component, Directive, Input} from '@angular/core'; +import {ComponentFixture, TestBed, async} from '@angular/core/testing'; +import {By} from '@angular/platform-browser/src/dom/debug/by'; +import {expect} from '@angular/platform-browser/testing/matchers'; + +export function main() { + describe('integration tests', () => { + let fixture: ComponentFixture; + + + describe('directives', () => { + it('should support dotted selectors', async(() => { + @Directive({selector: '[dot.name]'}) + class MyDir { + @Input('dot.name') value: string; + } + + TestBed.configureTestingModule({ + declarations: [ + MyDir, + TestComponent, + ], + }); + + const template = `
`; + fixture = createTestComponent(template); + fixture.detectChanges(); + const myDir = fixture.debugElement.query(By.directive(MyDir)).injector.get(MyDir); + expect(myDir.value).toEqual('foo'); + })); + }); + + }); +} + +@Component({selector: 'test-cmp', template: ''}) +class TestComponent { +} + +function createTestComponent(template: string): ComponentFixture { + return TestBed.overrideComponent(TestComponent, {set: {template: template}}) + .createComponent(TestComponent); +} diff --git a/modules/@angular/compiler/test/selector_spec.ts b/modules/@angular/compiler/test/selector_spec.ts index 5a47e41829..c39a1d51b8 100644 --- a/modules/@angular/compiler/test/selector_spec.ts +++ b/modules/@angular/compiler/test/selector_spec.ts @@ -7,6 +7,7 @@ */ import {CssSelector, SelectorMatcher} from '@angular/compiler/src/selector'; +import {createElementCssSelector} from '@angular/compiler/src/template_parser/template_parser'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {el} from '@angular/platform-browser/testing/browser_util'; @@ -30,14 +31,14 @@ export function main() { it('should select by element name case sensitive', () => { matcher.addSelectables(s1 = CssSelector.parse('someTag'), 1); - expect(matcher.match(CssSelector.parse('SOMEOTHERTAG')[0], selectableCollector)) + expect(matcher.match(getSelectorFor({tag: 'SOMEOTHERTAG'}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect(matcher.match(CssSelector.parse('SOMETAG')[0], selectableCollector)).toEqual(false); + expect(matcher.match(getSelectorFor({tag: 'SOMETAG'}), selectableCollector)).toEqual(false); expect(matched).toEqual([]); - expect(matcher.match(CssSelector.parse('someTag')[0], selectableCollector)).toEqual(true); + expect(matcher.match(getSelectorFor({tag: 'someTag'}), selectableCollector)).toEqual(true); expect(matched).toEqual([s1[0], 1]); }); @@ -45,21 +46,22 @@ export function main() { matcher.addSelectables(s1 = CssSelector.parse('.someClass'), 1); matcher.addSelectables(s2 = CssSelector.parse('.someClass.class2'), 2); - expect(matcher.match(CssSelector.parse('.SOMEOTHERCLASS')[0], selectableCollector)) + expect(matcher.match(getSelectorFor({classes: 'SOMEOTHERCLASS'}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect(matcher.match(CssSelector.parse('.SOMECLASS')[0], selectableCollector)).toEqual(true); + expect(matcher.match(getSelectorFor({classes: 'SOMECLASS'}), selectableCollector)) + .toEqual(true); expect(matched).toEqual([s1[0], 1]); reset(); - expect(matcher.match(CssSelector.parse('.someClass.class2')[0], selectableCollector)) + expect(matcher.match(getSelectorFor({classes: 'someClass class2'}), selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1, s2[0], 2]); }); it('should not throw for class name "constructor"', () => { - expect(matcher.match(CssSelector.parse('.constructor')[0], selectableCollector)) + expect(matcher.match(getSelectorFor({classes: 'constructor'}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); }); @@ -68,36 +70,43 @@ export function main() { matcher.addSelectables(s1 = CssSelector.parse('[someAttr]'), 1); matcher.addSelectables(s2 = CssSelector.parse('[someAttr][someAttr2]'), 2); - expect(matcher.match(CssSelector.parse('[SOMEOTHERATTR]')[0], selectableCollector)) + expect(matcher.match(getSelectorFor({attrs: [['SOMEOTHERATTR', '']]}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect(matcher.match(CssSelector.parse('[SOMEATTR]')[0], selectableCollector)).toEqual(false); - expect(matched).toEqual([]); - - expect(matcher.match(CssSelector.parse('[SOMEATTR=someValue]')[0], selectableCollector)) + expect(matcher.match(getSelectorFor({attrs: [['SOMEATTR', '']]}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect(matcher.match(CssSelector.parse('[someAttr][someAttr2]')[0], selectableCollector)) + expect( + matcher.match(getSelectorFor({attrs: [['SOMEATTR', 'someValue']]}), selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + + expect( + matcher.match( + getSelectorFor({attrs: [['someAttr', ''], ['someAttr2', '']]}), selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1, s2[0], 2]); reset(); expect(matcher.match( - CssSelector.parse('[someAttr=someValue][someAttr2]')[0], selectableCollector)) + getSelectorFor({attrs: [['someAttr', 'someValue'], ['someAttr2', '']]}), + selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1, s2[0], 2]); reset(); expect(matcher.match( - CssSelector.parse('[someAttr2][someAttr=someValue]')[0], selectableCollector)) + getSelectorFor({attrs: [['someAttr2', ''], ['someAttr', 'someValue']]}), + selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1, s2[0], 2]); reset(); expect(matcher.match( - CssSelector.parse('[someAttr2=someValue][someAttr]')[0], selectableCollector)) + getSelectorFor({attrs: [['someAttr2', 'someValue'], ['someAttr', '']]}), + selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1, s2[0], 2]); }); @@ -105,11 +114,13 @@ export function main() { it('should support "." in attribute names', () => { matcher.addSelectables(s1 = CssSelector.parse('[foo.bar]'), 1); - expect(matcher.match(CssSelector.parse('[barfoo]')[0], selectableCollector)).toEqual(false); + expect(matcher.match(getSelectorFor({attrs: [['barfoo', '']]}), selectableCollector)) + .toEqual(false); expect(matched).toEqual([]); reset(); - expect(matcher.match(CssSelector.parse('[foo.bar]')[0], selectableCollector)).toEqual(true); + expect(matcher.match(getSelectorFor({attrs: [['foo.bar', '']]}), selectableCollector)) + .toEqual(true); expect(matched).toEqual([s1[0], 1]); }); @@ -127,15 +138,18 @@ export function main() { it('should select by attr name case sensitive and value case insensitive', () => { matcher.addSelectables(s1 = CssSelector.parse('[someAttr=someValue]'), 1); - expect(matcher.match(CssSelector.parse('[SOMEATTR=SOMEOTHERATTR]')[0], selectableCollector)) + expect(matcher.match( + getSelectorFor({attrs: [['SOMEATTR', 'SOMEOTHERATTR']]}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect(matcher.match(CssSelector.parse('[SOMEATTR=SOMEVALUE]')[0], selectableCollector)) + expect( + matcher.match(getSelectorFor({attrs: [['SOMEATTR', 'SOMEVALUE']]}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect(matcher.match(CssSelector.parse('[someAttr=SOMEVALUE]')[0], selectableCollector)) + expect( + matcher.match(getSelectorFor({attrs: [['someAttr', 'SOMEVALUE']]}), selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1]); }); @@ -143,31 +157,38 @@ export function main() { it('should select by element name, class name and attribute name with value', () => { matcher.addSelectables(s1 = CssSelector.parse('someTag.someClass[someAttr=someValue]'), 1); + expect( + matcher.match( + getSelectorFor( + {tag: 'someOtherTag', classes: 'someOtherClass', attrs: [['someOtherAttr', '']]}), + selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + expect(matcher.match( - CssSelector.parse('someOtherTag.someOtherClass[someOtherAttr]')[0], + getSelectorFor( + {tag: 'someTag', classes: 'someOtherClass', attrs: [['someOtherAttr', '']]}), selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect( - matcher.match( - CssSelector.parse('someTag.someOtherClass[someOtherAttr]')[0], selectableCollector)) + expect(matcher.match( + getSelectorFor( + {tag: 'someTag', classes: 'someClass', attrs: [['someOtherAttr', '']]}), + selectableCollector)) .toEqual(false); expect(matched).toEqual([]); expect(matcher.match( - CssSelector.parse('someTag.someClass[someOtherAttr]')[0], selectableCollector)) + getSelectorFor({tag: 'someTag', classes: 'someClass', attrs: [['someAttr', '']]}), + selectableCollector)) .toEqual(false); expect(matched).toEqual([]); - expect( - matcher.match(CssSelector.parse('someTag.someClass[someAttr]')[0], selectableCollector)) - .toEqual(false); - expect(matched).toEqual([]); - - expect( - matcher.match( - CssSelector.parse('someTag.someClass[someAttr=someValue]')[0], selectableCollector)) + expect(matcher.match( + getSelectorFor( + {tag: 'someTag', classes: 'someClass', attrs: [['someAttr', 'someValue']]}), + selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1]); }); @@ -217,7 +238,9 @@ export function main() { matcher.addSelectables(CssSelector.parse(':not(p)'), 4); matcher.addSelectables(CssSelector.parse(':not(p[someAttr])'), 5); - expect(matcher.match(CssSelector.parse('p.someClass[someAttr]')[0], selectableCollector)) + expect(matcher.match( + getSelectorFor({tag: 'p', classes: 'someClass', attrs: [['someAttr', '']]}), + selectableCollector)) .toEqual(false); expect(matched).toEqual([]); }); @@ -228,32 +251,38 @@ export function main() { matcher.addSelectables(s3 = CssSelector.parse(':not(.someClass)'), 3); matcher.addSelectables(s4 = CssSelector.parse(':not(.someOtherClass[someAttr])'), 4); - expect(matcher.match( - CssSelector.parse('p[someOtherAttr].someOtherClass')[0], selectableCollector)) + expect( + matcher.match( + getSelectorFor({tag: 'p', attrs: [['someOtherAttr', '']], classes: 'someOtherClass'}), + selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1, s2[0], 2, s3[0], 3, s4[0], 4]); }); it('should match * with :not selector', () => { matcher.addSelectables(CssSelector.parse(':not([a])'), 1); - expect(matcher.match(CssSelector.parse('div')[0], () => {})).toEqual(true); + expect(matcher.match(getSelectorFor({tag: 'div'}), () => {})).toEqual(true); }); it('should match with multiple :not selectors', () => { matcher.addSelectables(s1 = CssSelector.parse('div:not([a]):not([b])'), 1); - expect(matcher.match(CssSelector.parse('div[a]')[0], selectableCollector)).toBe(false); - expect(matcher.match(CssSelector.parse('div[b]')[0], selectableCollector)).toBe(false); - expect(matcher.match(CssSelector.parse('div[c]')[0], selectableCollector)).toBe(true); + expect(matcher.match(getSelectorFor({tag: 'div', attrs: [['a', '']]}), selectableCollector)) + .toBe(false); + expect(matcher.match(getSelectorFor({tag: 'div', attrs: [['b', '']]}), selectableCollector)) + .toBe(false); + expect(matcher.match(getSelectorFor({tag: 'div', attrs: [['c', '']]}), selectableCollector)) + .toBe(true); }); it('should select with one match in a list', () => { matcher.addSelectables(s1 = CssSelector.parse('input[type=text], textbox'), 1); - expect(matcher.match(CssSelector.parse('textbox')[0], selectableCollector)).toEqual(true); + expect(matcher.match(getSelectorFor({tag: 'textbox'}), selectableCollector)).toEqual(true); expect(matched).toEqual([s1[1], 1]); reset(); - expect(matcher.match(CssSelector.parse('input[type=text]')[0], selectableCollector)) + expect(matcher.match( + getSelectorFor({tag: 'input', attrs: [['type', 'text']]}), selectableCollector)) .toEqual(true); expect(matched).toEqual([s1[0], 1]); }); @@ -261,7 +290,8 @@ export function main() { it('should not select twice with two matches in a list', () => { matcher.addSelectables(s1 = CssSelector.parse('input, .someClass'), 1); - expect(matcher.match(CssSelector.parse('input.someclass')[0], selectableCollector)) + expect( + matcher.match(getSelectorFor({tag: 'input', classes: 'someclass'}), selectableCollector)) .toEqual(true); expect(matched.length).toEqual(2); expect(matched).toEqual([s1[0], 1]); @@ -404,3 +434,16 @@ export function main() { }); }); } + +function getSelectorFor( + {tag = '', attrs = [], classes = ''}: {tag?: string, attrs?: any[], classes?: string} = {}): + CssSelector { + const selector = new CssSelector(); + selector.setElement(tag); + + attrs.forEach(nameValue => { selector.addAttribute(nameValue[0], nameValue[1]); }); + + classes.trim().split(/\s+/g).forEach(cName => { selector.addClassName(cName); }); + + return selector; +} \ No newline at end of file diff --git a/modules/@angular/compiler/test/template_parser/template_parser_spec.ts b/modules/@angular/compiler/test/template_parser/template_parser_spec.ts index e179c7d9dc..9ef6fe4818 100644 --- a/modules/@angular/compiler/test/template_parser/template_parser_spec.ts +++ b/modules/@angular/compiler/test/template_parser/template_parser_spec.ts @@ -623,6 +623,23 @@ Binding to attribute 'onEvent' is disallowed for security reasons (" { + const dirA = + CompileDirectiveMetadata + .create({ + selector: '[dot.name]', + type: createTypeMeta({reference: {filePath: someModuleUrl, name: 'DirA'}}), + inputs: ['localName: dot.name'], + }) + .toSummary(); + + expect(humanizeTplAst(parse('
', [dirA]))).toEqual([ + [ElementAst, 'div'], + [DirectiveAst, dirA], + [BoundDirectivePropertyAst, 'localName', 'expr'], + ]); + }); + it('should locate directives in property bindings', () => { const dirA = CompileDirectiveMetadata @@ -1244,8 +1261,15 @@ Reference "#a" is defined several times ("
]#a>
}); it('should parse variables via let ...', () => { - expect(humanizeTplAst(parse('
', [ - ]))).toEqual([[EmbeddedTemplateAst], [VariableAst, 'a', 'b'], [ElementAst, 'div']]); + const targetAst = [ + [EmbeddedTemplateAst], + [VariableAst, 'a', 'b'], + [ElementAst, 'div'], + ]; + + expect(humanizeTplAst(parse('
', []))).toEqual(targetAst); + + expect(humanizeTplAst(parse('
', []))).toEqual(targetAst); }); describe('directives', () => { diff --git a/modules/@angular/upgrade/src/util.ts b/modules/@angular/upgrade/src/util.ts index fb577f8cae..e0568ced8e 100644 --- a/modules/@angular/upgrade/src/util.ts +++ b/modules/@angular/upgrade/src/util.ts @@ -21,9 +21,9 @@ export function controllerKey(name: string): string { return '$' + name + 'Controller'; } -export function getAttributesAsArray(node: Node): string[][] { +export function getAttributesAsArray(node: Node): [string, string][] { const attributes = node.attributes; - let asArray: string[][]; + let asArray: [string, string][]; if (attributes) { let attrLen = attributes.length; asArray = new Array(attrLen);