Skip to content

Commit 9506cdf

Browse files
committed
fix(compiler-cli): infer type of event target for void elements (angular#62648)
Currently we infer the target of DOM events to be `EventTarget | null` which is consistent with the built-in types for `addEventListener`. This is due to the fact that users can dispatch custom events, or the event might've bubbled. However, this typing is also inconvenient for some other common use cases like `<input (input)="query($event.target.value)">`, because we don't have the ability to type cast in a template. These changes aim to make some of the cases simpler by inferring the type of `$event.target` if the event is bound on a void element which guarantees that it couldn't have bubbled. PR Close angular#62648
1 parent 2ab638d commit 9506cdf

File tree

3 files changed

+101
-2
lines changed

3 files changed

+101
-2
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1642,8 +1642,9 @@ class TcbUnclaimedOutputsOp extends TcbOp {
16421642
// `HTMLElement.addEventListener` using `HTMLElementEventMap` to infer an accurate type for
16431643
// `$event` depending on the event name. For unknown event names, TypeScript resorts to the
16441644
// base `Event` type.
1645-
const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer);
16461645
let target: ts.Expression;
1646+
let domEventAssertion: ts.Expression | undefined;
1647+
16471648
// Only check for `window` and `document` since in theory any target can be passed.
16481649
if (output.target === 'window' || output.target === 'document') {
16491650
target = ts.factory.createIdentifier(output.target);
@@ -1652,11 +1653,52 @@ class TcbUnclaimedOutputsOp extends TcbOp {
16521653
} else {
16531654
target = elId;
16541655
}
1656+
1657+
// By default the target of an event is `EventTarget | null`, because of bubbling
1658+
// and custom events. This can be inconvenient in some common cases like `input` elements
1659+
// since we don't have the ability to type cast in templates. We can improve the type
1660+
// checking for some of these cases by inferring the target based on the element it was
1661+
// bound to. We can only do this safely if the element is a void element (e.g. `input` or
1662+
// `img`), because we know that it couldn't have bubbled from a child. The event handler
1663+
// with the assertion would look as follows:
1664+
//
1665+
// ```
1666+
// const _t1 = document.createElement('input');
1667+
//
1668+
// _t1.addEventListener('input', ($event) => {
1669+
// ɵassertType<typeof _t1>($event.target);
1670+
// handler($event.target);
1671+
// });
1672+
// ```
1673+
if (
1674+
this.target instanceof TmplAstElement &&
1675+
this.target.isVoid &&
1676+
ts.isIdentifier(target)
1677+
) {
1678+
domEventAssertion = ts.factory.createCallExpression(
1679+
this.tcb.env.referenceExternalSymbol('@angular/core', 'ɵassertType'),
1680+
[ts.factory.createTypeQueryNode(target)],
1681+
[
1682+
ts.factory.createPropertyAccessExpression(
1683+
ts.factory.createIdentifier(EVENT_PARAMETER),
1684+
'target',
1685+
),
1686+
],
1687+
);
1688+
}
1689+
16551690
const propertyAccess = ts.factory.createPropertyAccessExpression(
16561691
target,
16571692
'addEventListener',
16581693
);
16591694
addParseSpanInfo(propertyAccess, output.keySpan);
1695+
const handler = tcbCreateEventHandler(
1696+
output,
1697+
this.tcb,
1698+
this.scope,
1699+
EventParamType.Infer,
1700+
domEventAssertion,
1701+
);
16601702
const call = ts.factory.createCallExpression(
16611703
/* expression */ propertyAccess,
16621704
/* typeArguments */ undefined,
@@ -3476,10 +3518,15 @@ function tcbCreateEventHandler(
34763518
tcb: Context,
34773519
scope: Scope,
34783520
eventType: EventParamType | ts.TypeNode,
3521+
assertionExpression?: ts.Expression,
34793522
): ts.Expression {
34803523
const handler = tcbEventHandlerExpression(event.handler, tcb, scope);
34813524
const statements: ts.Statement[] = [];
34823525

3526+
if (assertionExpression !== undefined) {
3527+
statements.push(ts.factory.createExpressionStatement(assertionExpression));
3528+
}
3529+
34833530
// TODO(crisbeto): remove the `checkTwoWayBoundEvents` check in v20.
34843531
if (event.type === ParsedEventType.TwoWay && tcb.env.config.checkTwoWayBoundEvents) {
34853532
// If we're dealing with a two-way event, we create a variable initialized to the unwrapped

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ describe('type check blocks', () => {
4141
expect(tcb(TEMPLATE)).toContain('(-1)');
4242
});
4343

44+
it('should assert the type for DOM events bound on void elements', () => {
45+
const result = tcb(`<input (input)="handleInput($event.target.value)">`);
46+
expect(result).toContain('i1.ɵassertType<typeof _t1>($event.target);');
47+
expect(result).toContain('(this).handleInput((((($event).target)).value));');
48+
});
49+
4450
it('should handle keyed property access', () => {
4551
const TEMPLATE = `{{a[b]}}`;
4652
expect(tcb(TEMPLATE)).toContain('(((this).a))[((this).b)]');
@@ -371,7 +377,7 @@ describe('type check blocks', () => {
371377
const block = tcb(TEMPLATE);
372378
expect(block).not.toContain('"div"');
373379
expect(block).toContain(
374-
'var _t2 = document.createElement("button"); ' + 'var _t1 = _t2; ' + '_t2.addEventListener',
380+
'var _t1 = document.createElement("button"); ' + 'var _t2 = _t1; ' + '_t1.addEventListener',
375381
);
376382
});
377383

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8492,5 +8492,51 @@ suppress
84928492
expect(diags.length).toBe(0);
84938493
});
84948494
});
8495+
8496+
describe('DOM event target type inference', () => {
8497+
beforeEach(() => {
8498+
env.tsconfig({strictTemplates: true});
8499+
});
8500+
8501+
it('should infer the type of the event target when bound on a void element', () => {
8502+
env.write(
8503+
'test.ts',
8504+
`
8505+
import {Component} from '@angular/core';
8506+
8507+
@Component({template: '<input (input)="handleEvent($event.target)">'})
8508+
export class TestCmp {
8509+
handleEvent(value: string) {}
8510+
}
8511+
`,
8512+
);
8513+
8514+
const diags = env.driveDiagnostics();
8515+
expect(diags.length).toBe(1);
8516+
expect(diags[0].messageText).toBe(
8517+
`Argument of type 'HTMLInputElement' is not assignable to parameter of type 'string'.`,
8518+
);
8519+
});
8520+
8521+
it('should not infer the type of the event target when bound on a non-void element', () => {
8522+
env.write(
8523+
'test.ts',
8524+
`
8525+
import {Component} from '@angular/core';
8526+
8527+
@Component({template: '<div (click)="handleEvent($event.target)"></div>'})
8528+
export class TestCmp {
8529+
handleEvent(value: string) {}
8530+
}
8531+
`,
8532+
);
8533+
8534+
const diags = env.driveDiagnostics();
8535+
expect(diags.length).toBe(1);
8536+
expect((diags[0].messageText as ts.DiagnosticMessageChain).messageText).toBe(
8537+
`Argument of type 'EventTarget | null' is not assignable to parameter of type 'string'.`,
8538+
);
8539+
});
8540+
});
84958541
});
84968542
});

0 commit comments

Comments
 (0)