From 46ea6843512a3f64f4ee61f9497ee34d76979562 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 17 Dec 2020 12:15:44 -0800 Subject: [PATCH] refactor(compiler-cli): find symbol for output when there is a two way binding (#40185) This commit fixes the Template Type Checker's `getSymbolOfNode` so that it is able to retrieve a symbol for the `BoundEvent` of a two-way binding. Previously, the implementation would locate the node in the TCB for the input because it appeared first and shares the same `keySpan` as the event binding. To fix this, the TCB node search now verifies that the located node matches the expected name for the output subscription: either `addEventListener` for a native listener or the class member of the Angular `@Output` in the case of an Angular output, as would be the case for two-way bindings. PR Close #40185 --- .../typecheck/src/template_symbol_builder.ts | 45 +++++++++++++---- ...ecker__get_symbol_of_template_node_spec.ts | 49 +++++++++++++++++++ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts index d3fcc2f5c4..c509777c49 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts @@ -168,24 +168,49 @@ export class SymbolBuilder { } private getSymbolOfBoundEvent(eventBinding: TmplAstBoundEvent): OutputBindingSymbol|null { + const consumer = this.templateData.boundTarget.getConsumerOfBinding(eventBinding); + if (consumer === null) { + return null; + } + // Outputs in the TCB look like one of the two: // * _outputHelper(_t1["outputField"]).subscribe(handler); // * _t1.addEventListener(handler); // Even with strict null checks disabled, we still produce the access as a separate statement // so that it can be found here. - const outputFieldAccesses = findAllMatchingNodes( - this.typeCheckBlock, {withSpan: eventBinding.keySpan, filter: isAccessExpression}); + let expectedAccess: string; + if (consumer instanceof TmplAstTemplate || consumer instanceof TmplAstElement) { + expectedAccess = 'addEventListener'; + } else { + const bindingPropertyNames = consumer.outputs.getByBindingPropertyName(eventBinding.name); + if (bindingPropertyNames === null || bindingPropertyNames.length === 0) { + return null; + } + // Note that we only get the expectedAccess text from a single consumer of the binding. If + // there are multiple consumers (not supported in the `boundTarget` API) and one of them has + // an alias, it will not get matched here. + expectedAccess = bindingPropertyNames[0].classPropertyName; + } + + function filter(n: ts.Node): n is ts.PropertyAccessExpression|ts.ElementAccessExpression { + if (!isAccessExpression(n)) { + return false; + } + + if (ts.isPropertyAccessExpression(n)) { + return n.name.getText() === expectedAccess; + } else { + return ts.isStringLiteral(n.argumentExpression) && + n.argumentExpression.text === expectedAccess; + } + } + const outputFieldAccesses = + findAllMatchingNodes(this.typeCheckBlock, {withSpan: eventBinding.keySpan, filter}); const bindings: BindingSymbol[] = []; for (const outputFieldAccess of outputFieldAccesses) { - const consumer = this.templateData.boundTarget.getConsumerOfBinding(eventBinding); - if (consumer === null) { - continue; - } - if (consumer instanceof TmplAstTemplate || consumer instanceof TmplAstElement) { - if (!ts.isPropertyAccessExpression(outputFieldAccess) || - outputFieldAccess.name.text !== 'addEventListener') { + if (!ts.isPropertyAccessExpression(outputFieldAccess)) { continue; } @@ -233,10 +258,10 @@ export class SymbolBuilder { }); } } + if (bindings.length === 0) { return null; } - return {kind: SymbolKind.Output, bindings}; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index 850b3494b9..a70c8bd09c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -1348,6 +1348,55 @@ runInEachFileSystem(() => { .parent.name?.text) .toEqual('TestDir'); }); + + + it('returns output symbol for two way binding', () => { + const fileName = absoluteFrom('/main.ts'); + const dirFile = absoluteFrom('/dir.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: {'Cmp': `
`}, + source: ` + export class Cmp { + value = ''; + }`, + declarations: [ + { + name: 'TestDir', + selector: '[dir]', + file: dirFile, + type: 'directive', + inputs: {ngModel: 'ngModel'}, + outputs: {ngModelChange: 'ngModelChange'}, + }, + ] + }, + { + fileName: dirFile, + source: ` + export class TestDir { + ngModel!: string; + ngModelChange!: EventEmitter; + }`, + templates: {}, + } + ]); + const sf = getSourceFileOrError(program, fileName); + const cmp = getClass(sf, 'Cmp'); + + const nodes = templateTypeChecker.getTemplate(cmp)!; + + const outputABinding = (nodes[0] as TmplAstElement).outputs[0]; + const symbol = templateTypeChecker.getSymbolOfNode(outputABinding, cmp)!; + assertOutputBindingSymbol(symbol); + expect( + (symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration).name.getText()) + .toEqual('ngModelChange'); + expect((symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration) + .parent.name?.text) + .toEqual('TestDir'); + }); }); describe('for elements', () => {