diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 08589c143c..327311a1dd 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -869,7 +869,7 @@ export class ParsedProperty { constructor( public name: string, public expression: ASTWithSource, public type: ParsedPropertyType, - // TODO(atscott): `keySpan` should really be required but allows `undefined` so VE does + // TODO(FW-2095): `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) { @@ -897,7 +897,8 @@ export class ParsedEvent { constructor( public name: string, public targetOrPhase: string, public type: ParsedEventType, public handler: ASTWithSource, public sourceSpan: ParseSourceSpan, - public handlerSpan: ParseSourceSpan) {} + // TODO(FW-2095): keySpan should be required but was made optional to avoid changing VE + public handlerSpan: ParseSourceSpan, readonly keySpan: ParseSourceSpan|undefined) {} } /** diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts index 0d408651bb..68153667b1 100644 --- a/packages/compiler/src/render3/r3_ast.ts +++ b/packages/compiler/src/render3/r3_ast.ts @@ -73,14 +73,19 @@ export class BoundEvent implements Node { constructor( public name: string, public type: ParsedEventType, public handler: AST, public target: string|null, public phase: string|null, public sourceSpan: ParseSourceSpan, - public handlerSpan: ParseSourceSpan) {} + public handlerSpan: ParseSourceSpan, readonly keySpan: ParseSourceSpan) {} static fromParsedEvent(event: ParsedEvent) { const target: string|null = event.type === ParsedEventType.Regular ? event.targetOrPhase : null; const phase: string|null = event.type === ParsedEventType.Animation ? event.targetOrPhase : null; + if (event.keySpan === undefined) { + throw new Error(`Unexpected state: keySpan must be defined for bound event but was not for ${ + event.name}: ${event.sourceSpan}`); + } return new BoundEvent( - event.name, event.type, event.handler, target, phase, event.sourceSpan, event.handlerSpan); + event.name, event.type, event.handler, target, phase, event.sourceSpan, event.handlerSpan, + event.keySpan); } 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 a30a3c67d3..3019bfb3b3 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -361,9 +361,10 @@ class HtmlAstToIvyAst implements html.Visitor { } else if (bindParts[KW_ON_IDX]) { const events: ParsedEvent[] = []; const identifier = bindParts[IDENT_KW_IDX]; + const keySpan = createKeySpan(srcSpan, bindParts[KW_ON_IDX], identifier); this.bindingParser.parseEvent( - identifier, value, srcSpan, attribute.valueSpan || srcSpan, matchableAttributes, - events); + identifier, value, srcSpan, attribute.valueSpan || srcSpan, matchableAttributes, events, + keySpan); addEvents(events, boundEvents); } else if (bindParts[KW_BINDON_IDX]) { const identifier = bindParts[IDENT_KW_IDX]; @@ -372,7 +373,8 @@ class HtmlAstToIvyAst implements html.Visitor { identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan, matchableAttributes, parsedProperties, keySpan); this.parseAssignmentEvent( - identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents); + identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents, + keySpan); } else if (bindParts[KW_AT_IDX]) { const keySpan = createKeySpan(srcSpan, '', name); this.bindingParser.parseLiteralAttr( @@ -399,23 +401,23 @@ class HtmlAstToIvyAst implements html.Visitor { // TODO(ayazhafiz): update this to handle malformed bindings. name.endsWith(delims.end) && name.length > delims.start.length + delims.end.length) { const identifier = name.substring(delims.start.length, name.length - delims.end.length); + const keySpan = createKeySpan(srcSpan, delims.start, identifier); if (delims.start === BINDING_DELIMS.BANANA_BOX.start) { - const keySpan = createKeySpan(srcSpan, delims.start, identifier); this.bindingParser.parsePropertyBinding( identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan, matchableAttributes, parsedProperties, keySpan); this.parseAssignmentEvent( - identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents); + identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents, + keySpan); } else if (delims.start === BINDING_DELIMS.PROPERTY.start) { - const keySpan = createKeySpan(srcSpan, delims.start, identifier); this.bindingParser.parsePropertyBinding( identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan, matchableAttributes, parsedProperties, keySpan); } else { const events: ParsedEvent[] = []; this.bindingParser.parseEvent( - identifier, value, srcSpan, attribute.valueSpan || srcSpan, matchableAttributes, - events); + identifier, value, srcSpan, attribute.valueSpan || srcSpan, matchableAttributes, events, + keySpan); addEvents(events, boundEvents); } @@ -463,11 +465,11 @@ class HtmlAstToIvyAst implements html.Visitor { private parseAssignmentEvent( name: string, expression: string, sourceSpan: ParseSourceSpan, valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][], - boundEvents: t.BoundEvent[]) { + boundEvents: t.BoundEvent[], keySpan: ParseSourceSpan) { const events: ParsedEvent[] = []; this.bindingParser.parseEvent( `${name}Change`, `${expression}=$event`, sourceSpan, valueSpan || sourceSpan, - targetMatchableAttrs, events); + targetMatchableAttrs, events, keySpan); addEvents(events, boundEvents); } diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index f810d55099..ea3636673a 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -97,8 +97,14 @@ export class BindingParser { Object.keys(dirMeta.hostListeners).forEach(propName => { const expression = dirMeta.hostListeners[propName]; if (typeof expression === 'string') { - // TODO: pass a more accurate handlerSpan for this event. - this.parseEvent(propName, expression, sourceSpan, sourceSpan, [], targetEvents); + // Use the `sourceSpan` for `keySpan` and `handlerSpan`. 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. + this.parseEvent( + propName, expression, sourceSpan, sourceSpan, [], targetEvents, sourceSpan); } else { this._reportError( `Value of the host listener "${ @@ -408,19 +414,20 @@ export class BindingParser { boundProp.sourceSpan, boundProp.keySpan, boundProp.valueSpan); } + // TODO: keySpan should be required but was made optional to avoid changing VE parser. parseEvent( name: string, expression: string, sourceSpan: ParseSourceSpan, handlerSpan: ParseSourceSpan, - targetMatchableAttrs: string[][], targetEvents: ParsedEvent[]) { + targetMatchableAttrs: string[][], targetEvents: ParsedEvent[], keySpan?: ParseSourceSpan) { if (name.length === 0) { this._reportError(`Event name is missing in binding`, sourceSpan); } if (isAnimationLabel(name)) { name = name.substr(1); - this._parseAnimationEvent(name, expression, sourceSpan, handlerSpan, targetEvents); + this._parseAnimationEvent(name, expression, sourceSpan, handlerSpan, targetEvents, keySpan); } else { this._parseRegularEvent( - name, expression, sourceSpan, handlerSpan, targetMatchableAttrs, targetEvents); + name, expression, sourceSpan, handlerSpan, targetMatchableAttrs, targetEvents, keySpan); } } @@ -432,7 +439,7 @@ export class BindingParser { private _parseAnimationEvent( name: string, expression: string, sourceSpan: ParseSourceSpan, handlerSpan: ParseSourceSpan, - targetEvents: ParsedEvent[]) { + targetEvents: ParsedEvent[], keySpan?: ParseSourceSpan) { const matches = splitAtPeriod(name, [name, '']); const eventName = matches[0]; const phase = matches[1].toLowerCase(); @@ -442,7 +449,7 @@ export class BindingParser { case 'done': const ast = this._parseAction(expression, handlerSpan); targetEvents.push(new ParsedEvent( - eventName, phase, ParsedEventType.Animation, ast, sourceSpan, handlerSpan)); + eventName, phase, ParsedEventType.Animation, ast, sourceSpan, handlerSpan, keySpan)); break; default: @@ -462,13 +469,13 @@ export class BindingParser { private _parseRegularEvent( name: string, expression: string, sourceSpan: ParseSourceSpan, handlerSpan: ParseSourceSpan, - targetMatchableAttrs: string[][], targetEvents: ParsedEvent[]) { + targetMatchableAttrs: string[][], targetEvents: ParsedEvent[], keySpan?: ParseSourceSpan) { // long format: 'target: eventName' const [target, eventName] = splitAtColon(name, [null!, name]); const ast = this._parseAction(expression, handlerSpan); targetMatchableAttrs.push([name!, ast.source!]); - targetEvents.push( - new ParsedEvent(eventName, target, ParsedEventType.Regular, ast, sourceSpan, handlerSpan)); + targetEvents.push(new ParsedEvent( + eventName, target, ParsedEventType.Regular, ast, sourceSpan, handlerSpan, keySpan)); // Don't detect directives for event names for now, // so don't add the event name to the matchableAttrs } diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts index e7e34c07ff..3b1ef79825 100644 --- a/packages/compiler/test/render3/r3_ast_spans_spec.ts +++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts @@ -78,8 +78,10 @@ class R3AstSourceSpans implements t.Visitor { } visitBoundEvent(event: t.BoundEvent) { - this.result.push( - ['BoundEvent', humanizeSpan(event.sourceSpan), humanizeSpan(event.handlerSpan)]); + this.result.push([ + 'BoundEvent', humanizeSpan(event.sourceSpan), humanizeSpan(event.keySpan), + humanizeSpan(event.handlerSpan) + ]); } visitText(text: t.Text) { @@ -356,21 +358,21 @@ describe('R3 AST source spans', () => { it('is correct for event names case sensitive', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundEvent', '(someEvent)="v"', 'v'], + ['BoundEvent', '(someEvent)="v"', 'someEvent', 'v'], ]); }); it('is correct for bound events via on-', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundEvent', 'on-event="v"', 'v'], + ['BoundEvent', 'on-event="v"', 'event', 'v'], ]); }); it('is correct for bound events via data-on-', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], - ['BoundEvent', 'data-on-event="v"', 'v'], + ['BoundEvent', 'data-on-event="v"', 'event', 'v'], ]); }); @@ -378,7 +380,7 @@ describe('R3 AST source spans', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], ['BoundAttribute', '[(prop)]="v"', 'prop', 'v'], - ['BoundEvent', '[(prop)]="v"', 'v'], + ['BoundEvent', '[(prop)]="v"', 'prop', 'v'], ]); }); @@ -386,7 +388,7 @@ describe('R3 AST source spans', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], ['BoundAttribute', 'bindon-prop="v"', 'prop', 'v'], - ['BoundEvent', 'bindon-prop="v"', 'v'], + ['BoundEvent', 'bindon-prop="v"', 'prop', 'v'], ]); }); @@ -394,7 +396,7 @@ describe('R3 AST source spans', () => { expectFromHtml('
').toEqual([ ['Element', '
', '
', '
'], ['BoundAttribute', 'data-bindon-prop="v"', 'prop', 'v'], - ['BoundEvent', 'data-bindon-prop="v"', 'v'], + ['BoundEvent', 'data-bindon-prop="v"', 'prop', 'v'], ]); }); }); diff --git a/packages/language-service/ivy/test/definitions_spec.ts b/packages/language-service/ivy/test/definitions_spec.ts index 50f0dc59ed..117f572593 100644 --- a/packages/language-service/ivy/test/definitions_spec.ts +++ b/packages/language-service/ivy/test/definitions_spec.ts @@ -199,7 +199,7 @@ describe('definitions', () => { it('should work for event providers', () => { const definitions = getDefinitionsAndAssertBoundSpan({ templateOverride: ``, - expectedSpanText: '(test)="myClick($event)"', + expectedSpanText: 'test', }); expect(definitions!.length).toEqual(1); @@ -218,7 +218,7 @@ describe('definitions', () => { it('should return the directive when the event is part of the selector', () => { const definitions = getDefinitionsAndAssertBoundSpan({ templateOverride: `
`, - expectedSpanText: `(eventSelector)="title = ''"`, + expectedSpanText: `eventSelector`, }); expect(definitions!.length).toEqual(2); diff --git a/packages/language-service/ivy/test/quick_info_spec.ts b/packages/language-service/ivy/test/quick_info_spec.ts index 75963e389e..b8a6b5b6d6 100644 --- a/packages/language-service/ivy/test/quick_info_spec.ts +++ b/packages/language-service/ivy/test/quick_info_spec.ts @@ -156,7 +156,7 @@ describe('quick info', () => { it('should work for event providers', () => { expectQuickInfo({ templateOverride: ``, - expectedSpanText: '(test)="myClick($event)"', + expectedSpanText: 'test', expectedDisplayString: '(event) TestComponent.testEvent: EventEmitter' }); }); @@ -164,12 +164,12 @@ describe('quick info', () => { it('should work for on- syntax binding', () => { expectQuickInfo({ templateOverride: ``, - expectedSpanText: 'on-test="myClick($event)"', + expectedSpanText: 'test', expectedDisplayString: '(event) TestComponent.testEvent: EventEmitter' }); expectQuickInfo({ templateOverride: ``, - expectedSpanText: 'data-on-test="myClick($event)"', + expectedSpanText: 'test', expectedDisplayString: '(event) TestComponent.testEvent: EventEmitter' }); });