Skip to content

Commit

Permalink
refactor(compiler-cli): produce binding access when checkTypeOfOutput…
Browse files Browse the repository at this point in the history
…Events is false (#39515)

When `checkTypeOfOutputEvents` is `false`, we still need to produce the access
to the `EventEmitter` so the Language Service can still get the
type information about the field. That is, in a template `<div
(output)="handle($event)"`, we still want to be able to grab information
when the cursor is inside the "output" parens. The flag is intended only
to affect whether the compiler produces diagnostics for the inferred
type of the `$event`.

PR Close #39515
  • Loading branch information
atscott authored and alxhub committed Dec 10, 2020
1 parent 702d6bf commit 269a775
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,11 @@ export class SymbolBuilder {
}

private getSymbolOfBoundEvent(eventBinding: TmplAstBoundEvent): OutputBindingSymbol|null {
// Outputs are a `ts.CallExpression` that look like one of the two:
// 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 outputFieldAccess = findFirstMatchingNode(
this.typeCheckBlock, {withSpan: eventBinding.keySpan, filter: isAccessExpression});
if (outputFieldAccess === null) {
Expand Down
20 changes: 12 additions & 8 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,11 @@ export class TcbDirectiveOutputsOp extends TcbOp {
// TODO(alxhub): consider supporting multiple fields with the same property name for outputs.
const field = outputs.getByBindingPropertyName(output.name)![0].classPropertyName;

if (dirId === null) {
dirId = this.scope.resolve(this.node, this.dir);
}
const outputField = ts.createElementAccess(dirId, ts.createStringLiteral(field));
addParseSpanInfo(outputField, output.keySpan);
if (this.tcb.env.config.checkTypeOfOutputEvents) {
// For strict checking of directive events, generate a call to the `subscribe` method
// on the directive's output field to let type information flow into the handler function's
Expand All @@ -877,21 +882,20 @@ export class TcbDirectiveOutputsOp extends TcbOp {
// specially crafted set of signatures, to effectively cast `EventEmitter<T>` to something
// that has a `subscribe` method that properly carries the `T` into the handler function.
const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer);

if (dirId === null) {
dirId = this.scope.resolve(this.node, this.dir);
}
const outputField = ts.createElementAccess(dirId, ts.createStringLiteral(field));
addParseSpanInfo(outputField, output.keySpan);
const outputHelper =
ts.createCall(this.tcb.env.declareOutputHelper(), undefined, [outputField]);
const subscribeFn = ts.createPropertyAccess(outputHelper, 'subscribe');
const call = ts.createCall(subscribeFn, /* typeArguments */ undefined, [handler]);
addParseSpanInfo(call, output.sourceSpan);
this.scope.addStatement(ts.createExpressionStatement(call));
} else {
// If strict checking of directive events is disabled, emit a handler function where the
// `$event` parameter has an explicit `any` type.
// If strict checking of directive events is disabled:
//
// * We still generate the access to the output field as a statement in the TCB so consumers
// of the `TemplateTypeChecker` can still find the node for the class member for the
// output.
// * Emit a handler function where the `$event` parameter has an explicit `any` type.
this.scope.addStatement(ts.createExpressionStatement(outputField));
const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Any);
this.scope.addStatement(ts.createExpressionStatement(handler));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ describe('type check blocks', () => {
expect(block).toContain('function ($event: any): any { (ctx).foo($event); }');
// Note that DOM events are still checked, that is controlled by `checkTypeOfDomEvents`
expect(block).toContain(
'_t1.addEventListener("nonDirOutput", function ($event): any { (ctx).foo($event); });');
'addEventListener("nonDirOutput", function ($event): any { (ctx).foo($event); });');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ runInEachFileSystem(() => {
assertExpressionSymbol(eventSymbol);
});

it('returns empty list when checkTypeOfOutputEvents is false', () => {
it('still returns binding when checkTypeOfOutputEvents is false', () => {
const fileName = absoluteFrom('/main.ts');
const dirFile = absoluteFrom('/dir.ts');
const {program, templateTypeChecker} = setup(
Expand Down Expand Up @@ -1302,9 +1302,14 @@ runInEachFileSystem(() => {
const nodes = templateTypeChecker.getTemplate(cmp)!;

const outputABinding = (nodes[0] as TmplAstElement).outputs[0];
const symbol = templateTypeChecker.getSymbolOfNode(outputABinding, cmp);
// TODO(atscott): should type checker still generate the subscription in this case?
expect(symbol).toBeNull();
const symbol = templateTypeChecker.getSymbolOfNode(outputABinding, cmp)!;
assertOutputBindingSymbol(symbol);
expect(
(symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration).name.getText())
.toEqual('outputA');
expect((symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration)
.parent.name?.text)
.toEqual('TestDir');
});
});

Expand Down
11 changes: 11 additions & 0 deletions packages/language-service/ivy/test/quick_info_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,17 @@ describe('quick info', () => {
expectedDisplayString: '(property) TestComponent.name: string'
});
});

it('can still get quick info when strictOutputEventTypes is false', () => {
initMockFileSystem('Native');
env = LanguageServiceTestEnvironment.setup(
quickInfoSkeleton(), {strictOutputEventTypes: false});
expectQuickInfo({
templateOverride: `<test-comp (te¦st)="myClick($event)"></test-comp>`,
expectedSpanText: 'test',
expectedDisplayString: '(event) TestComponent.testEvent: EventEmitter<string>'
});
});
});

function expectQuickInfo(
Expand Down

0 comments on commit 269a775

Please sign in to comment.