feat(ivy): better error messages for unknown components (#33064)

For elements in a template that look like custom elements, i.e.
containing a dash in their name, the template type checker will now
issue an error with instructions on how the resolve the issue.
Additionally, a property binding to a non-existent property will also
produce a more descriptive error message.

Resolves FW-1597

PR Close #33064
This commit is contained in:
JoostK 2019-10-09 21:00:00 +02:00 committed by Miško Hevery
parent c88305d2eb
commit d8249d1230
3 changed files with 83 additions and 10 deletions

View File

@ -61,7 +61,7 @@ export interface DomSchemaChecker {
* Checks non-Angular elements and properties against the `DomElementSchemaRegistry`, a schema * Checks non-Angular elements and properties against the `DomElementSchemaRegistry`, a schema
* maintained by the Angular team via extraction from a browser IDL. * maintained by the Angular team via extraction from a browser IDL.
*/ */
export class RegistryDomSchemaChecker { export class RegistryDomSchemaChecker implements DomSchemaChecker {
private _diagnostics: ts.Diagnostic[] = []; private _diagnostics: ts.Diagnostic[] = [];
get diagnostics(): ReadonlyArray<ts.Diagnostic> { return this._diagnostics; } get diagnostics(): ReadonlyArray<ts.Diagnostic> { return this._diagnostics; }
@ -71,9 +71,21 @@ export class RegistryDomSchemaChecker {
checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void { checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void {
if (!REGISTRY.hasElement(element.name, schemas)) { if (!REGISTRY.hasElement(element.name, schemas)) {
const mapping = this.resolver.getSourceMapping(id); const mapping = this.resolver.getSourceMapping(id);
let errorMsg = `'${element.name}' is not a known element:\n`;
errorMsg +=
`1. If '${element.name}' is an Angular component, then verify that it is part of this module.\n`;
if (element.name.indexOf('-') > -1) {
errorMsg +=
`2. If '${element.name}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`;
} else {
errorMsg +=
`2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`;
}
const diag = makeTemplateDiagnostic( const diag = makeTemplateDiagnostic(
mapping, element.sourceSpan, ts.DiagnosticCategory.Error, mapping, element.sourceSpan, ts.DiagnosticCategory.Error,
ErrorCode.SCHEMA_INVALID_ELEMENT, `'${element.name}' is not a valid HTML element.`); ErrorCode.SCHEMA_INVALID_ELEMENT, errorMsg);
this._diagnostics.push(diag); this._diagnostics.push(diag);
} }
} }
@ -83,9 +95,22 @@ export class RegistryDomSchemaChecker {
schemas: SchemaMetadata[]): void { schemas: SchemaMetadata[]): void {
if (!REGISTRY.hasProperty(element.name, name, schemas)) { if (!REGISTRY.hasProperty(element.name, name, schemas)) {
const mapping = this.resolver.getSourceMapping(id); const mapping = this.resolver.getSourceMapping(id);
let errorMsg =
`Can't bind to '${name}' since it isn't a known property of '${element.name}'.`;
if (element.name.startsWith('ng-')) {
errorMsg +=
`\n1. If '${name}' is an Angular directive, then add 'CommonModule' to the '@NgModule.imports' of this component.` +
`\n2. To allow any property add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`;
} else if (element.name.indexOf('-') > -1) {
errorMsg +=
`\n1. If '${element.name}' is an Angular component and it has '${name}' input, then verify that it is part of this module.` +
`\n2. If '${element.name}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.` +
`\n3. To allow any property add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`;
}
const diag = makeTemplateDiagnostic( const diag = makeTemplateDiagnostic(
mapping, span, ts.DiagnosticCategory.Error, ErrorCode.SCHEMA_INVALID_ATTRIBUTE, mapping, span, ts.DiagnosticCategory.Error, ErrorCode.SCHEMA_INVALID_ATTRIBUTE, errorMsg);
`'${name}' is not a valid property of <${element.name}>.`);
this._diagnostics.push(diag); this._diagnostics.push(diag);
} }
} }

View File

@ -150,7 +150,7 @@ runInEachFileSystem(() => {
expect(messages).toEqual([ expect(messages).toEqual([
`synthetic.html(1, 29): Property 'heihgt' does not exist on type 'TestComponent'. Did you mean 'height'?`, `synthetic.html(1, 29): Property 'heihgt' does not exist on type 'TestComponent'. Did you mean 'height'?`,
`synthetic.html(1, 6): 'srcc' is not a valid property of <img>.`, `synthetic.html(1, 6): Can't bind to 'srcc' since it isn't a known property of 'img'.`,
]); ]);
}); });

View File

@ -8,7 +8,7 @@
import * as ts from 'typescript'; import * as ts from 'typescript';
import {absoluteFrom as _} from '../../src/ngtsc/file_system'; import {absoluteFrom as _, getFileSystem} from '../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing'; import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
import {loadStandardTestFiles} from '../helpers/src/mock_file_loading'; import {loadStandardTestFiles} from '../helpers/src/mock_file_loading';
@ -365,7 +365,29 @@ export declare class CommonModule {
`); `);
const diags = env.driveDiagnostics(); const diags = env.driveDiagnostics();
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].messageText).toBe(`'foo' is not a valid HTML element.`); expect(diags[0].messageText).toBe(`'foo' is not a known element:
1. If 'foo' is an Angular component, then verify that it is part of this module.
2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`);
});
it('should have a descriptive error for unknown elements that contain a dash', () => {
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'blah',
template: '<my-foo>test</my-foo>',
})
export class FooCmp {}
@NgModule({
declarations: [FooCmp],
})
export class FooModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toBe(`'my-foo' is not a known element:
1. If 'my-foo' is an Angular component, then verify that it is part of this module.
2. If 'my-foo' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`);
}); });
it('should check for unknown properties', () => { it('should check for unknown properties', () => {
@ -383,7 +405,27 @@ export declare class CommonModule {
`); `);
const diags = env.driveDiagnostics(); const diags = env.driveDiagnostics();
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
expect(diags[0].messageText).toBe(`'foo' is not a valid property of <div>.`); expect(diags[0].messageText)
.toBe(`Can't bind to 'foo' since it isn't a known property of 'div'.`);
});
it('should have a descriptive error for unknown properties with an "ng-" prefix', () => {
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'blah',
template: '<div [foo]="1">test</div>',
})
export class FooCmp {}
@NgModule({
declarations: [FooCmp],
})
export class FooModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toBe(`Can't bind to 'foo' since it isn't a known property of 'div'.`);
}); });
it('should convert property names when binding special properties', () => { it('should convert property names when binding special properties', () => {
@ -423,8 +465,14 @@ export declare class CommonModule {
`); `);
const diags = env.driveDiagnostics(); const diags = env.driveDiagnostics();
expect(diags.length).toBe(2); expect(diags.length).toBe(2);
expect(diags[0].messageText).toBe(`'custom-element' is not a valid HTML element.`); expect(diags[0].messageText).toBe(`'custom-element' is not a known element:
expect(diags[1].messageText).toBe(`'foo' is not a valid property of <custom-element>.`); 1. If 'custom-element' is an Angular component, then verify that it is part of this module.
2. If 'custom-element' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`);
expect(diags[1].messageText)
.toBe(`Can't bind to 'foo' since it isn't a known property of 'custom-element'.
1. If 'custom-element' is an Angular component and it has 'foo' input, then verify that it is part of this module.
2. If 'custom-element' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.
3. To allow any property add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`);
}); });
it('should not produce diagnostics for custom-elements-style elements when using the CUSTOM_ELEMENTS_SCHEMA', it('should not produce diagnostics for custom-elements-style elements when using the CUSTOM_ELEMENTS_SCHEMA',