From d8249d1230a939b58ebcabe47c90fc18febd2c2a Mon Sep 17 00:00:00 2001 From: JoostK Date: Wed, 9 Oct 2019 21:00:00 +0200 Subject: [PATCH] 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 --- .../src/ngtsc/typecheck/src/dom.ts | 33 +++++++++-- .../ngtsc/typecheck/test/diagnostics_spec.ts | 2 +- .../test/ngtsc/template_typecheck_spec.ts | 58 +++++++++++++++++-- 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts index 798a3e8288..73d67ad68b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts @@ -61,7 +61,7 @@ export interface DomSchemaChecker { * Checks non-Angular elements and properties against the `DomElementSchemaRegistry`, a schema * maintained by the Angular team via extraction from a browser IDL. */ -export class RegistryDomSchemaChecker { +export class RegistryDomSchemaChecker implements DomSchemaChecker { private _diagnostics: ts.Diagnostic[] = []; get diagnostics(): ReadonlyArray { return this._diagnostics; } @@ -71,9 +71,21 @@ export class RegistryDomSchemaChecker { checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void { if (!REGISTRY.hasElement(element.name, schemas)) { 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( 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); } } @@ -83,9 +95,22 @@ export class RegistryDomSchemaChecker { schemas: SchemaMetadata[]): void { if (!REGISTRY.hasProperty(element.name, name, schemas)) { 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( - mapping, span, ts.DiagnosticCategory.Error, ErrorCode.SCHEMA_INVALID_ATTRIBUTE, - `'${name}' is not a valid property of <${element.name}>.`); + mapping, span, ts.DiagnosticCategory.Error, ErrorCode.SCHEMA_INVALID_ATTRIBUTE, errorMsg); this._diagnostics.push(diag); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 49cf660e73..255b6623f3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -150,7 +150,7 @@ runInEachFileSystem(() => { expect(messages).toEqual([ `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 .`, + `synthetic.html(1, 6): Can't bind to 'srcc' since it isn't a known property of 'img'.`, ]); }); diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index a3be05aa27..cff3cd7382 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -8,7 +8,7 @@ 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 {loadStandardTestFiles} from '../helpers/src/mock_file_loading'; @@ -365,7 +365,29 @@ export declare class CommonModule { `); const diags = env.driveDiagnostics(); 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: 'test', + }) + 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', () => { @@ -383,7 +405,27 @@ export declare class CommonModule { `); const diags = env.driveDiagnostics(); expect(diags.length).toBe(1); - expect(diags[0].messageText).toBe(`'foo' is not a valid property of
.`); + 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: '
test
', + }) + 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', () => { @@ -423,8 +465,14 @@ export declare class CommonModule { `); const diags = env.driveDiagnostics(); expect(diags.length).toBe(2); - expect(diags[0].messageText).toBe(`'custom-element' is not a valid HTML element.`); - expect(diags[1].messageText).toBe(`'foo' is not a valid property of .`); + expect(diags[0].messageText).toBe(`'custom-element' is not a known 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',