Skip to content

Commit 3e37e89

Browse files
atscottalxhub
authored andcommitted
fix(language-service): provide dom event completions (#43299)
Native DOM events were previously not included in the completions because the dom schema registry would filter out events completely. This change updates the registry to include events in the private element->property map and excludes events from lookups outside of the new `allKnownEventsOfElement` function. fixes angular/vscode-ng-language-service#1479 PR Close #43299
1 parent 353cad2 commit 3e37e89

File tree

6 files changed

+88
-9
lines changed

6 files changed

+88
-9
lines changed

packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@ export interface TemplateTypeChecker {
153153
*/
154154
getPotentialDomBindings(tagName: string): {attribute: string, property: string}[];
155155

156+
/**
157+
* Retrieve any potential DOM events.
158+
*/
159+
getPotentialDomEvents(tagName: string): string[];
160+
156161
/**
157162
* Retrieve the type checking engine's metadata for the given directive class, if available.
158163
*/

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,10 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
582582
}));
583583
}
584584

585+
getPotentialDomEvents(tagName: string): string[] {
586+
return REGISTRY.allKnownEventsOfElement(tagName);
587+
}
588+
585589
private getScopeData(component: ts.ClassDeclaration): ScopeData|null {
586590
if (this.scopeCache.has(component)) {
587591
return this.scopeCache.get(component)!;

packages/compiler/src/schema/dom_element_schema_registry.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {dashCaseToCamelCase} from '../util';
1414
import {SECURITY_SCHEMA} from './dom_security_schema';
1515
import {ElementSchemaRegistry} from './element_schema_registry';
1616

17+
const EVENT = 'event';
1718
const BOOLEAN = 'boolean';
1819
const NUMBER = 'number';
1920
const STRING = 'string';
@@ -249,30 +250,36 @@ const _PROP_TO_ATTR: {[name: string]: string} =
249250

250251
export class DomElementSchemaRegistry extends ElementSchemaRegistry {
251252
private _schema: {[element: string]: {[property: string]: string}} = {};
253+
// We don't allow binding to events for security reasons. Allowing event bindings would almost
254+
// certainly introduce bad XSS vulnerabilities. Instead, we store events in a separate schema.
255+
private _eventSchema: {[element: string]: Set<string>} = {};
252256

253257
constructor() {
254258
super();
255259
SCHEMA.forEach(encodedType => {
256260
const type: {[property: string]: string} = {};
261+
const events: Set<string> = new Set();
257262
const [strType, strProperties] = encodedType.split('|');
258263
const properties = strProperties.split(',');
259264
const [typeNames, superName] = strType.split('^');
260-
typeNames.split(',').forEach(tag => this._schema[tag.toLowerCase()] = type);
265+
typeNames.split(',').forEach(tag => {
266+
this._schema[tag.toLowerCase()] = type;
267+
this._eventSchema[tag.toLowerCase()] = events;
268+
});
261269
const superType = superName && this._schema[superName.toLowerCase()];
262270
if (superType) {
263271
Object.keys(superType).forEach((prop: string) => {
264272
type[prop] = superType[prop];
265273
});
274+
for (const superEvent of this._eventSchema[superName.toLowerCase()]) {
275+
events.add(superEvent);
276+
}
266277
}
267278
properties.forEach((property: string) => {
268279
if (property.length > 0) {
269280
switch (property[0]) {
270281
case '*':
271-
// We don't yet support events.
272-
// If ever allowing to bind to events, GO THROUGH A SECURITY REVIEW, allowing events
273-
// will
274-
// almost certainly introduce bad XSS vulnerabilities.
275-
// type[property.substring(1)] = EVENT;
282+
events.add(property.substring(1));
276283
break;
277284
case '!':
278285
type[property.substring(1)] = BOOLEAN;
@@ -400,6 +407,10 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
400407
return Object.keys(elementProperties).map(prop => _PROP_TO_ATTR[prop] ?? prop);
401408
}
402409

410+
allKnownEventsOfElement(tagName: string): string[] {
411+
return Array.from(this._eventSchema[tagName.toLowerCase()] ?? []);
412+
}
413+
403414
override normalizeAnimationStyleProperty(propName: string): string {
404415
return dashCaseToCamelCase(propName);
405416
}

packages/language-service/ivy/attribute_completions.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ export enum AttributeCompletionKind {
3232
*/
3333
DomProperty,
3434

35+
/**
36+
* Completion of an event from the DOM schema.
37+
*/
38+
DomEvent,
39+
3540
/**
3641
* Completion of an attribute that results in a new directive being matched on an element.
3742
*/
@@ -87,6 +92,15 @@ export interface DomPropertyCompletion {
8792
property: string;
8893
}
8994

95+
export interface DomEventCompletion {
96+
kind: AttributeCompletionKind.DomEvent;
97+
98+
/**
99+
* Name of the DOM event
100+
*/
101+
eventName: string;
102+
}
103+
90104
/**
91105
* Completion of an attribute which results in a new directive being matched on an element.
92106
*/
@@ -163,8 +177,9 @@ export interface DirectiveOutputCompletion {
163177
*
164178
* Disambiguated by the `kind` property into various types of completions.
165179
*/
166-
export type AttributeCompletion = DomAttributeCompletion|DomPropertyCompletion|
167-
DirectiveAttributeCompletion|DirectiveInputCompletion|DirectiveOutputCompletion;
180+
export type AttributeCompletion =
181+
DomAttributeCompletion|DomPropertyCompletion|DirectiveAttributeCompletion|
182+
DirectiveInputCompletion|DirectiveOutputCompletion|DomEventCompletion;
168183

169184
/**
170185
* Given an element and its context, produce a `Map` of all possible attribute completions.
@@ -345,8 +360,13 @@ export function buildAttributeCompletionTable(
345360
});
346361
}
347362
}
363+
for (const event of checker.getPotentialDomEvents(element.name)) {
364+
table.set(event, {
365+
kind: AttributeCompletionKind.DomEvent,
366+
eventName: event,
367+
});
368+
}
348369
}
349-
350370
return table;
351371
}
352372

@@ -466,6 +486,16 @@ export function addAttributeCompletionEntries(
466486
replacementSpan,
467487
});
468488
}
489+
break;
490+
}
491+
case AttributeCompletionKind.DomEvent: {
492+
entries.push({
493+
kind: unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.EVENT),
494+
name: `(${completion.eventName})`,
495+
sortText: completion.eventName,
496+
replacementSpan,
497+
});
498+
break;
469499
}
470500
}
471501
}
@@ -474,6 +504,7 @@ export function getAttributeCompletionSymbol(
474504
completion: AttributeCompletion, checker: ts.TypeChecker): ts.Symbol|null {
475505
switch (completion.kind) {
476506
case AttributeCompletionKind.DomAttribute:
507+
case AttributeCompletionKind.DomEvent:
477508
case AttributeCompletionKind.DomProperty:
478509
return null;
479510
case AttributeCompletionKind.DirectiveAttribute:

packages/language-service/ivy/completions.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,11 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
568568
// the user is completing on a property binding `[foo|]`, don't offer output event
569569
// completions.
570570
switch (completion.kind) {
571+
case AttributeCompletionKind.DomEvent:
572+
if (this.node instanceof TmplAstBoundAttribute) {
573+
continue;
574+
}
575+
break;
571576
case AttributeCompletionKind.DomAttribute:
572577
case AttributeCompletionKind.DomProperty:
573578
if (this.node instanceof TmplAstBoundEvent) {
@@ -644,6 +649,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
644649
let documentation: ts.SymbolDisplayPart[]|undefined = undefined;
645650
let info: DisplayInfo|null;
646651
switch (completion.kind) {
652+
case AttributeCompletionKind.DomEvent:
647653
case AttributeCompletionKind.DomAttribute:
648654
case AttributeCompletionKind.DomProperty:
649655
// TODO(alxhub): ideally we would show the same documentation as quick info here. However,

packages/language-service/ivy/test/completions_spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,21 @@ describe('completions', () => {
506506
['[value]']);
507507
});
508508

509+
it('should return event completion', () => {
510+
const {templateFile} = setup(`<button ></button>`, ``);
511+
templateFile.moveCursorToText(`<button ¦>`);
512+
const completions = templateFile.getCompletionsAtPosition();
513+
expectContain(completions, DisplayInfoKind.EVENT, ['(click)']);
514+
});
515+
516+
it('should return event completion with empty parens', () => {
517+
const {templateFile} = setup(`<button ()></button>`, ``);
518+
templateFile.moveCursorToText(`<button (¦)>`);
519+
const completions = templateFile.getCompletionsAtPosition();
520+
expectContain(completions, DisplayInfoKind.EVENT, ['(click)']);
521+
});
522+
523+
509524
it('should return completions for a partial attribute', () => {
510525
const {appFile} = setupInlineTemplate(`<input val>`, '');
511526
appFile.moveCursorToText('<input val¦>');
@@ -536,6 +551,13 @@ describe('completions', () => {
536551
['value']);
537552
expectReplacementText(completions, appFile.contents, 'val');
538553
});
554+
555+
it('should return completions inside an event binding', () => {
556+
const {templateFile} = setup(`<button (cl)=''></button>`, ``);
557+
templateFile.moveCursorToText(`(cl¦)=''`);
558+
const completions = templateFile.getCompletionsAtPosition();
559+
expectContain(completions, DisplayInfoKind.EVENT, ['(click)']);
560+
});
539561
});
540562

541563
describe('directive present', () => {

0 commit comments

Comments
 (0)