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
This commit is contained in:
Andrew Scott 2020-12-17 12:15:44 -08:00 committed by atscott
parent 104546569e
commit 46ea684351
2 changed files with 84 additions and 10 deletions

View File

@ -168,24 +168,49 @@ export class SymbolBuilder {
} }
private getSymbolOfBoundEvent(eventBinding: TmplAstBoundEvent): OutputBindingSymbol|null { 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: // Outputs in the TCB look like one of the two:
// * _outputHelper(_t1["outputField"]).subscribe(handler); // * _outputHelper(_t1["outputField"]).subscribe(handler);
// * _t1.addEventListener(handler); // * _t1.addEventListener(handler);
// Even with strict null checks disabled, we still produce the access as a separate statement // Even with strict null checks disabled, we still produce the access as a separate statement
// so that it can be found here. // so that it can be found here.
const outputFieldAccesses = findAllMatchingNodes( let expectedAccess: string;
this.typeCheckBlock, {withSpan: eventBinding.keySpan, filter: isAccessExpression}); 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[] = []; const bindings: BindingSymbol[] = [];
for (const outputFieldAccess of outputFieldAccesses) { for (const outputFieldAccess of outputFieldAccesses) {
const consumer = this.templateData.boundTarget.getConsumerOfBinding(eventBinding);
if (consumer === null) {
continue;
}
if (consumer instanceof TmplAstTemplate || consumer instanceof TmplAstElement) { if (consumer instanceof TmplAstTemplate || consumer instanceof TmplAstElement) {
if (!ts.isPropertyAccessExpression(outputFieldAccess) || if (!ts.isPropertyAccessExpression(outputFieldAccess)) {
outputFieldAccess.name.text !== 'addEventListener') {
continue; continue;
} }
@ -233,10 +258,10 @@ export class SymbolBuilder {
}); });
} }
} }
if (bindings.length === 0) { if (bindings.length === 0) {
return null; return null;
} }
return {kind: SymbolKind.Output, bindings}; return {kind: SymbolKind.Output, bindings};
} }

View File

@ -1348,6 +1348,55 @@ runInEachFileSystem(() => {
.parent.name?.text) .parent.name?.text)
.toEqual('TestDir'); .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': `<div dir [(ngModel)]="value"></div>`},
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<string>;
}`,
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', () => { describe('for elements', () => {