diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 6568d6fafb..2773a5458d 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -841,7 +841,10 @@ export class ParsedProperty { constructor( public name: string, public expression: ASTWithSource, public type: ParsedPropertyType, - public sourceSpan: ParseSourceSpan, public valueSpan?: ParseSourceSpan) { + // TODO(atscott): `keySpan` should really be required but allows `undefined` so VE does + // not need to be updated. Make `keySpan` required when VE is removed. + public sourceSpan: ParseSourceSpan, readonly keySpan: ParseSourceSpan|undefined, + public valueSpan: ParseSourceSpan|undefined) { this.isLiteral = this.type === ParsedPropertyType.LITERAL_ATTR; this.isAnimation = this.type === ParsedPropertyType.ANIMATION; } @@ -896,5 +899,5 @@ export class BoundElementProperty { constructor( public name: string, public type: BindingType, public securityContext: SecurityContext, public value: ASTWithSource, public unit: string|null, public sourceSpan: ParseSourceSpan, - public valueSpan?: ParseSourceSpan) {} + readonly keySpan: ParseSourceSpan|undefined, public valueSpan: ParseSourceSpan|undefined) {} } diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts index 996dacb5bd..33a86c629b 100644 --- a/packages/compiler/src/render3/r3_ast.ts +++ b/packages/compiler/src/render3/r3_ast.ts @@ -43,12 +43,18 @@ export class BoundAttribute implements Node { constructor( public name: string, public type: BindingType, public securityContext: SecurityContext, public value: AST, public unit: string|null, public sourceSpan: ParseSourceSpan, - public valueSpan?: ParseSourceSpan, public i18n?: I18nMeta) {} + readonly keySpan: ParseSourceSpan, public valueSpan: ParseSourceSpan|undefined, + public i18n: I18nMeta|undefined) {} - static fromBoundElementProperty(prop: BoundElementProperty, i18n?: I18nMeta) { + static fromBoundElementProperty(prop: BoundElementProperty, i18n?: I18nMeta): BoundAttribute { + if (prop.keySpan === undefined) { + throw new Error( + `Unexpected state: keySpan must be defined for bound attributes but was not for ${ + prop.name}: ${prop.sourceSpan}`); + } return new BoundAttribute( prop.name, prop.type, prop.securityContext, prop.value, prop.unit, prop.sourceSpan, - prop.valueSpan, i18n); + prop.keySpan, prop.valueSpan, i18n); } visit(visitor: Visitor): Result { diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts index 16d899a173..61feb3bf4a 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -332,15 +332,26 @@ class HtmlAstToIvyAst implements html.Visitor { const absoluteOffset = attribute.valueSpan ? attribute.valueSpan.start.offset : srcSpan.start.offset; + function createKeySpan(srcSpan: ParseSourceSpan, prefix: string, identifier: string) { + // We need to adjust the start location for the keySpan to account for the removed 'data-' + // prefix from `normalizeAttributeName`. + const normalizationAdjustment = attribute.name.length - name.length; + const keySpanStart = srcSpan.start.moveBy(prefix.length + normalizationAdjustment); + const keySpanEnd = keySpanStart.moveBy(identifier.length); + return new ParseSourceSpan(keySpanStart, keySpanEnd, identifier); + } + const bindParts = name.match(BIND_NAME_REGEXP); let hasBinding = false; if (bindParts) { hasBinding = true; if (bindParts[KW_BIND_IDX] != null) { + const identifier = bindParts[IDENT_KW_IDX]; + const keySpan = createKeySpan(srcSpan, bindParts[KW_BIND_IDX], identifier); this.bindingParser.parsePropertyBinding( - bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, attribute.valueSpan, - matchableAttributes, parsedProperties); + identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan, + matchableAttributes, parsedProperties, keySpan); } else if (bindParts[KW_LET_IDX]) { if (isTemplateElement) { @@ -353,37 +364,41 @@ class HtmlAstToIvyAst implements html.Visitor { } else if (bindParts[KW_REF_IDX]) { const identifier = bindParts[IDENT_KW_IDX]; this.parseReference(identifier, value, srcSpan, attribute.valueSpan, references); - } else if (bindParts[KW_ON_IDX]) { const events: ParsedEvent[] = []; + const identifier = bindParts[IDENT_KW_IDX]; this.bindingParser.parseEvent( - bindParts[IDENT_KW_IDX], value, srcSpan, attribute.valueSpan || srcSpan, - matchableAttributes, events); + identifier, value, srcSpan, attribute.valueSpan || srcSpan, matchableAttributes, + events); addEvents(events, boundEvents); } else if (bindParts[KW_BINDON_IDX]) { + const identifier = bindParts[IDENT_KW_IDX]; + const keySpan = createKeySpan(srcSpan, bindParts[KW_BINDON_IDX], identifier); this.bindingParser.parsePropertyBinding( - bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, attribute.valueSpan, - matchableAttributes, parsedProperties); + identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan, + matchableAttributes, parsedProperties, keySpan); this.parseAssignmentEvent( - bindParts[IDENT_KW_IDX], value, srcSpan, attribute.valueSpan, matchableAttributes, - boundEvents); + identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents); } else if (bindParts[KW_AT_IDX]) { + const keySpan = createKeySpan(srcSpan, '', name); this.bindingParser.parseLiteralAttr( name, value, srcSpan, absoluteOffset, attribute.valueSpan, matchableAttributes, - parsedProperties); + parsedProperties, keySpan); } else if (bindParts[IDENT_BANANA_BOX_IDX]) { + const keySpan = createKeySpan(srcSpan, '[(', bindParts[IDENT_BANANA_BOX_IDX]); this.bindingParser.parsePropertyBinding( bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, absoluteOffset, - attribute.valueSpan, matchableAttributes, parsedProperties); + attribute.valueSpan, matchableAttributes, parsedProperties, keySpan); this.parseAssignmentEvent( bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents); } else if (bindParts[IDENT_PROPERTY_IDX]) { + const keySpan = createKeySpan(srcSpan, '[', bindParts[IDENT_PROPERTY_IDX]); this.bindingParser.parsePropertyBinding( bindParts[IDENT_PROPERTY_IDX], value, false, srcSpan, absoluteOffset, - attribute.valueSpan, matchableAttributes, parsedProperties); + attribute.valueSpan, matchableAttributes, parsedProperties, keySpan); } else if (bindParts[IDENT_EVENT_IDX]) { const events: ParsedEvent[] = []; @@ -393,8 +408,10 @@ class HtmlAstToIvyAst implements html.Visitor { addEvents(events, boundEvents); } } else { + const keySpan = createKeySpan(srcSpan, '' /* prefix */, name); hasBinding = this.bindingParser.parsePropertyInterpolation( - name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties); + name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties, + keySpan); } return hasBinding; diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 9844830d0a..790a14df23 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -582,9 +582,8 @@ function createHostBindingsFunction( // bindings with pipes. These calculates happen after this block. let totalHostVarsCount = 0; bindings && bindings.forEach((binding: ParsedProperty) => { - const name = binding.name; - const stylingInputWasSet = - styleBuilder.registerInputBasedOnName(name, binding.expression, binding.sourceSpan); + const stylingInputWasSet = styleBuilder.registerInputBasedOnName( + binding.name, binding.expression, hostBindingSourceSpan); if (stylingInputWasSet) { totalHostVarsCount += MIN_STYLING_BINDING_SLOTS_REQUIRED; } else { diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index ca0001f44e..1cbe4530ab 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -62,7 +62,13 @@ export class BindingParser { if (typeof expression === 'string') { this.parsePropertyBinding( propName, expression, true, sourceSpan, sourceSpan.start.offset, undefined, [], - boundProps); + // Use the `sourceSpan` for `keySpan`. This isn't really accurate, but neither is the + // sourceSpan, as it represents the sourceSpan of the host itself rather than the + // source of the host binding (which doesn't exist in the template). Regardless, + // neither of these values are used in Ivy but are only here to satisfy the function + // signature. This should likely be refactored in the future so that `sourceSpan` + // isn't being used inaccurately. + boundProps, sourceSpan); } else { this._reportError( `Value of the host property binding "${ @@ -155,14 +161,14 @@ export class BindingParser { } else if (binding.value) { const valueSpan = moveParseSourceSpan(sourceSpan, binding.value.ast.sourceSpan); this._parsePropertyAst( - key, binding.value, sourceSpan, valueSpan, targetMatchableAttrs, targetProps); + key, binding.value, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps); } else { targetMatchableAttrs.push([key, '' /* value */]); // Since this is a literal attribute with no RHS, source span should be // just the key span. this.parseLiteralAttr( key, null /* value */, keySpan, absoluteValueOffset, undefined /* valueSpan */, - targetMatchableAttrs, targetProps); + targetMatchableAttrs, targetProps, keySpan); } } } @@ -206,7 +212,9 @@ export class BindingParser { parseLiteralAttr( name: string, value: string|null, sourceSpan: ParseSourceSpan, absoluteOffset: number, valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][], - targetProps: ParsedProperty[]) { + // TODO(atscott): keySpan is only optional here so VE template parser implementation does not + // have to change This should be required when VE is removed. + targetProps: ParsedProperty[], keySpan?: ParseSourceSpan) { if (isAnimationLabel(name)) { name = name.substring(1); if (value) { @@ -216,18 +224,21 @@ export class BindingParser { sourceSpan, ParseErrorLevel.ERROR); } this._parseAnimation( - name, value, sourceSpan, absoluteOffset, valueSpan, targetMatchableAttrs, targetProps); + name, value, sourceSpan, absoluteOffset, keySpan, valueSpan, targetMatchableAttrs, + targetProps); } else { targetProps.push(new ParsedProperty( name, this._exprParser.wrapLiteralPrimitive(value, '', absoluteOffset), - ParsedPropertyType.LITERAL_ATTR, sourceSpan, valueSpan)); + ParsedPropertyType.LITERAL_ATTR, sourceSpan, keySpan, valueSpan)); } } parsePropertyBinding( name: string, expression: string, isHost: boolean, sourceSpan: ParseSourceSpan, absoluteOffset: number, valueSpan: ParseSourceSpan|undefined, - targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) { + // TODO(atscott): keySpan is only optional here so VE template parser implementation does not + // have to change This should be required when VE is removed. + targetMatchableAttrs: string[][], targetProps: ParsedProperty[], keySpan?: ParseSourceSpan) { if (name.length === 0) { this._reportError(`Property name is missing in binding`, sourceSpan); } @@ -243,22 +254,25 @@ export class BindingParser { if (isAnimationProp) { this._parseAnimation( - name, expression, sourceSpan, absoluteOffset, valueSpan, targetMatchableAttrs, + name, expression, sourceSpan, absoluteOffset, keySpan, valueSpan, targetMatchableAttrs, targetProps); } else { this._parsePropertyAst( name, this._parseBinding(expression, isHost, valueSpan || sourceSpan, absoluteOffset), - sourceSpan, valueSpan, targetMatchableAttrs, targetProps); + sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps); } } parsePropertyInterpolation( name: string, value: string, sourceSpan: ParseSourceSpan, valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][], - targetProps: ParsedProperty[]): boolean { + // TODO(atscott): keySpan is only optional here so VE template parser implementation does not + // have to change This should be required when VE is removed. + targetProps: ParsedProperty[], keySpan?: ParseSourceSpan): boolean { const expr = this.parseInterpolation(value, valueSpan || sourceSpan); if (expr) { - this._parsePropertyAst(name, expr, sourceSpan, valueSpan, targetMatchableAttrs, targetProps); + this._parsePropertyAst( + name, expr, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps); return true; } return false; @@ -266,17 +280,17 @@ export class BindingParser { private _parsePropertyAst( name: string, ast: ASTWithSource, sourceSpan: ParseSourceSpan, - valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][], - targetProps: ParsedProperty[]) { + keySpan: ParseSourceSpan|undefined, valueSpan: ParseSourceSpan|undefined, + targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) { targetMatchableAttrs.push([name, ast.source!]); targetProps.push( - new ParsedProperty(name, ast, ParsedPropertyType.DEFAULT, sourceSpan, valueSpan)); + new ParsedProperty(name, ast, ParsedPropertyType.DEFAULT, sourceSpan, keySpan, valueSpan)); } private _parseAnimation( name: string, expression: string|null, sourceSpan: ParseSourceSpan, absoluteOffset: number, - valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][], - targetProps: ParsedProperty[]) { + keySpan: ParseSourceSpan|undefined, valueSpan: ParseSourceSpan|undefined, + targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) { if (name.length === 0) { this._reportError('Animation trigger is missing', sourceSpan); } @@ -287,8 +301,8 @@ export class BindingParser { const ast = this._parseBinding( expression || 'undefined', false, valueSpan || sourceSpan, absoluteOffset); targetMatchableAttrs.push([name, ast.source!]); - targetProps.push( - new ParsedProperty(name, ast, ParsedPropertyType.ANIMATION, sourceSpan, valueSpan)); + targetProps.push(new ParsedProperty( + name, ast, ParsedPropertyType.ANIMATION, sourceSpan, keySpan, valueSpan)); } private _parseBinding( @@ -317,7 +331,7 @@ export class BindingParser { if (boundProp.isAnimation) { return new BoundElementProperty( boundProp.name, BindingType.Animation, SecurityContext.NONE, boundProp.expression, null, - boundProp.sourceSpan, boundProp.valueSpan); + boundProp.sourceSpan, boundProp.keySpan, boundProp.valueSpan); } let unit: string|null = null; @@ -370,7 +384,7 @@ export class BindingParser { return new BoundElementProperty( boundPropertyName, bindingType, securityContexts[0], boundProp.expression, unit, - boundProp.sourceSpan, boundProp.valueSpan); + boundProp.sourceSpan, boundProp.keySpan, boundProp.valueSpan); } parseEvent( diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts index fac18efac9..90268e5134 100644 --- a/packages/compiler/test/render3/r3_ast_spans_spec.ts +++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts @@ -65,8 +65,10 @@ class R3AstSourceSpans implements t.Visitor { } visitBoundAttribute(attribute: t.BoundAttribute) { - this.result.push( - ['BoundAttribute', humanizeSpan(attribute.sourceSpan), humanizeSpan(attribute.valueSpan)]); + this.result.push([ + 'BoundAttribute', humanizeSpan(attribute.sourceSpan), humanizeSpan(attribute.keySpan), + humanizeSpan(attribute.valueSpan) + ]); } visitBoundEvent(event: t.BoundEvent) { @@ -144,28 +146,35 @@ describe('R3 AST source spans', () => { it('is correct for bound properties', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundAttribute', '[someProp]="v"', 'v'], + ['BoundAttribute', '[someProp]="v"', 'someProp', 'v'], ]); }); it('is correct for bound properties without value', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundAttribute', '[someProp]', ''], + ['BoundAttribute', '[someProp]', 'someProp', ''], ]); }); it('is correct for bound properties via bind- ', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundAttribute', 'bind-prop="v"', 'v'], + ['BoundAttribute', 'bind-prop="v"', 'prop', 'v'], ]); }); it('is correct for bound properties via {{...}}', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundAttribute', 'prop="{{v}}"', '{{v}}'], + ['BoundAttribute', 'prop="{{v}}"', 'prop', '{{v}}'], + ]); + }); + + it('is correct for bound properties via data-', () => { + expectFromHtml('
').toEqual([ + ['Element', '
', '
', '
'], + ['BoundAttribute', 'data-prop="{{v}}"', 'prop', '{{v}}'], ]); }); }); @@ -208,6 +217,16 @@ describe('R3 AST source spans', () => { ]); }); + it('is correct for reference via data-ref-...', () => { + expectFromHtml('').toEqual([ + [ + 'Template', '', '', + '' + ], + ['Reference', 'data-ref-a', ''], + ]); + }); + it('is correct for variables via let-...', () => { expectFromHtml('').toEqual([ [ @@ -218,6 +237,16 @@ describe('R3 AST source spans', () => { ]); }); + it('is correct for variables via data-let-...', () => { + expectFromHtml('').toEqual([ + [ + 'Template', '', '', + '' + ], + ['Variable', 'data-let-a="b"', 'b'], + ]); + }); + it('is correct for attributes', () => { expectFromHtml('').toEqual([ [ @@ -234,7 +263,7 @@ describe('R3 AST source spans', () => { 'Template', '', '', '' ], - ['BoundAttribute', '[k1]="v1"', 'v1'], + ['BoundAttribute', '[k1]="v1"', 'k1', 'v1'], ]); }); }); @@ -252,7 +281,7 @@ describe('R3 AST source spans', () => { '' ], ['TextAttribute', 'ngFor', ''], - ['BoundAttribute', '*ngFor="let item of items"', 'items'], + ['BoundAttribute', '*ngFor="let item of items"', 'of', 'items'], ['Variable', 'let item ', ''], [ 'Element', '
', '
', @@ -270,10 +299,28 @@ describe('R3 AST source spans', () => { [ 'Template', '
', '
', '
' ], - ['BoundAttribute', '*ngFor="item of items"', 'item'], - ['BoundAttribute', '*ngFor="item of items"', 'items'], + ['BoundAttribute', '*ngFor="item of items"', 'ngFor', 'item'], + ['BoundAttribute', '*ngFor="item of items"', 'of', 'items'], ['Element', '
', '
', '
'], ]); + + expectFromHtml('
').toEqual([ + [ + 'Template', '
', + '
', '
' + ], + ['TextAttribute', 'ngFor', ''], + ['BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'of', 'items'], + [ + 'BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'trackBy', 'trackByFn' + ], + ['Variable', 'let item ', ''], + [ + 'Element', '
', + '
', '
' + ], + + ]); }); it('is correct for variables via let ...', () => { @@ -288,7 +335,7 @@ describe('R3 AST source spans', () => { it('is correct for variables via as ...', () => { expectFromHtml('
').toEqual([ ['Template', '
', '
', '
'], - ['BoundAttribute', '*ngIf="expr as local"', 'expr'], + ['BoundAttribute', '*ngIf="expr as local"', 'ngIf', 'expr'], ['Variable', 'ngIf="expr as local', 'ngIf'], ['Element', '
', '
', '
'], ]); @@ -310,10 +357,17 @@ describe('R3 AST source spans', () => { ]); }); + it('is correct for bound events via data-on-', () => { + expectFromHtml('
').toEqual([ + ['Element', '
', '
', '
'], + ['BoundEvent', 'data-on-event="v"', 'v'], + ]); + }); + it('is correct for bound events and properties via [(...)]', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundAttribute', '[(prop)]="v"', 'v'], + ['BoundAttribute', '[(prop)]="v"', 'prop', 'v'], ['BoundEvent', '[(prop)]="v"', 'v'], ]); }); @@ -321,10 +375,18 @@ describe('R3 AST source spans', () => { it('is correct for bound events and properties via bindon-', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundAttribute', 'bindon-prop="v"', 'v'], + ['BoundAttribute', 'bindon-prop="v"', 'prop', 'v'], ['BoundEvent', 'bindon-prop="v"', 'v'], ]); }); + + it('is correct for bound events and properties via data-bindon-', () => { + expectFromHtml('
').toEqual([ + ['Element', '
', '
', '
'], + ['BoundAttribute', 'data-bindon-prop="v"', 'prop', 'v'], + ['BoundEvent', 'data-bindon-prop="v"', 'v'], + ]); + }); }); describe('references', () => { @@ -348,5 +410,12 @@ describe('R3 AST source spans', () => { ['Reference', 'ref-a', ''], ]); }); + + it('is correct for references via data-ref-', () => { + expectFromHtml('
').toEqual([ + ['Element', '
', '
', '
'], + ['Reference', 'ref-a', ''], + ]); + }); }); });