From 00f3c58bb929d9afa10ab3c4f1b9ee7a0077ba57 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 3 Mar 2020 17:42:44 +0100 Subject: [PATCH] fix(core): log error instead of warning for unknown properties and elements (#35798) Changes the Ivy unknown element/property messages from being logged with `console.warn` to `console.error`. This should make them a bit more visible without breaking existing apps. Furthermore, a lot of folks filter out warning messages in the dev tools' console, whereas errors are usually still shown. Fixes #35699. PR Close #35798 --- .../core/src/render3/instructions/element.ts | 14 +++--- .../core/src/render3/instructions/shared.ts | 10 ++--- .../core/test/acceptance/ng_module_spec.ts | 44 +++++++++---------- packages/core/test/linker/integration_spec.ts | 12 ++--- .../test/linker/ng_module_integration_spec.ts | 4 +- .../test/linker/security_integration_spec.ts | 4 +- .../test/testing_public_spec.ts | 6 +-- 7 files changed, 47 insertions(+), 47 deletions(-) diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 52eda6249b..5c74054bde 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -36,7 +36,7 @@ function elementStartFirstCreatePass( const hasDirectives = resolveDirectives(tView, lView, tNode, getConstant(tViewConsts, localRefsIndex)); - ngDevMode && warnAboutUnknownElement(tView, lView, native, tNode, hasDirectives); + ngDevMode && logUnknownElementError(tView, lView, native, tNode, hasDirectives); if (tNode.mergedAttrs !== null) { computeStaticStyling(tNode, tNode.mergedAttrs); @@ -171,7 +171,7 @@ export function ɵɵelement( ɵɵelementEnd(); } -function warnAboutUnknownElement( +function logUnknownElementError( tView: TView, lView: LView, element: RElement, tNode: TNode, hasDirectives: boolean): void { const schemas = tView.schemas; @@ -197,17 +197,17 @@ function warnAboutUnknownElement( !customElements.get(tagName)); if (isUnknown && !matchingSchemas(tView, lView, tagName)) { - let warning = `'${tagName}' is not a known element:\n`; - warning += + let message = `'${tagName}' is not a known element:\n`; + message += `1. If '${tagName}' is an Angular component, then verify that it is part of this module.\n`; if (tagName && tagName.indexOf('-') > -1) { - warning += + message += `2. If '${tagName}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`; } else { - warning += + message += `2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`; } - console.warn(warning); + console.error(message); } } } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 8c0fa14d48..86e72d52ce 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -962,7 +962,7 @@ export function elementPropertyInternal( validateAgainstEventProperties(propName); if (!validateProperty(tView, lView, element, propName, tNode)) { // Return here since we only log warnings for unknown properties. - warnAboutUnknownProperty(propName, tNode); + logUnknownPropertyError(propName, tNode); return; } ngDevMode.rendererSetProperty++; @@ -982,7 +982,7 @@ export function elementPropertyInternal( // If the node is a container and the property didn't // match any of the inputs or schemas we should throw. if (ngDevMode && !matchingSchemas(tView, lView, tNode.tagName)) { - warnAboutUnknownProperty(propName, tNode); + logUnknownPropertyError(propName, tNode); } } } @@ -1070,12 +1070,12 @@ export function matchingSchemas(tView: TView, lView: LView, tagName: string | nu } /** - * Logs a warning that a property is not supported on an element. + * Logs an error that a property is not supported on an element. * @param propName Name of the invalid property. * @param tNode Node on which we encountered the property. */ -function warnAboutUnknownProperty(propName: string, tNode: TNode): void { - console.warn( +function logUnknownPropertyError(propName: string, tNode: TNode): void { + console.error( `Can't bind to '${propName}' since it isn't a known property of '${tNode.tagName}'.`); } diff --git a/packages/core/test/acceptance/ng_module_spec.ts b/packages/core/test/acceptance/ng_module_spec.ts index e23031c32e..7eaba6b08a 100644 --- a/packages/core/test/acceptance/ng_module_spec.ts +++ b/packages/core/test/acceptance/ng_module_spec.ts @@ -97,7 +97,7 @@ describe('NgModule', () => { }); describe('schemas', () => { - onlyInIvy('Unknown property warning logged instead of throwing an error') + onlyInIvy('Unknown property logs an error message instead of throwing') .it('should throw on unknown props if NO_ERRORS_SCHEMA is absent', () => { @Component({ selector: 'my-comp', @@ -120,7 +120,7 @@ describe('NgModule', () => { TestBed.configureTestingModule({imports: [MyModule]}); - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); const fixture = TestBed.createComponent(MyComp); fixture.detectChanges(); expect(spy.calls.mostRecent().args[0]) @@ -185,15 +185,15 @@ describe('NgModule', () => { }).not.toThrow(); }); - onlyInIvy('unknown element check logs a warning rather than throwing') - .it('should warn about unknown element without CUSTOM_ELEMENTS_SCHEMA for element with dash in tag name', + onlyInIvy('unknown element check logs an error message rather than throwing') + .it('should log an error about unknown element without CUSTOM_ELEMENTS_SCHEMA for element with dash in tag name', () => { @Component({template: ``}) class MyComp { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); TestBed.configureTestingModule({declarations: [MyComp]}); const fixture = TestBed.createComponent(MyComp); fixture.detectChanges(); @@ -215,14 +215,14 @@ describe('NgModule', () => { }).toThrowError(/'custom-el' is not a known element/); }); - onlyInIvy('unknown element check logs a warning rather than throwing') - .it('should warn about unknown element without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name', + onlyInIvy('unknown element check logs an error message rather than throwing') + .it('should log an error about unknown element without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name', () => { @Component({template: ``}) class MyComp { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); TestBed.configureTestingModule({declarations: [MyComp]}); const fixture = TestBed.createComponent(MyComp); fixture.detectChanges(); @@ -245,8 +245,8 @@ describe('NgModule', () => { }); onlyInIvy('test relies on Ivy-specific AOT format') - .it('should not log unknown element warning for AOT-compiled components', () => { - const spy = spyOn(console, 'warn'); + .it('should not log unknown element error for AOT-compiled components', () => { + const spy = spyOn(console, 'error'); /* * @Component({ @@ -310,13 +310,13 @@ describe('NgModule', () => { expect(spy).not.toHaveBeenCalled(); }); - onlyInIvy('unknown element check logs a warning rather than throwing') - .it('should not warn about unknown elements with CUSTOM_ELEMENTS_SCHEMA', () => { + onlyInIvy('unknown element check logs an error message rather than throwing') + .it('should not log an error about unknown elements with CUSTOM_ELEMENTS_SCHEMA', () => { @Component({template: ``}) class MyComp { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); TestBed.configureTestingModule({ declarations: [MyComp], schemas: [CUSTOM_ELEMENTS_SCHEMA], @@ -344,13 +344,13 @@ describe('NgModule', () => { }).not.toThrow(); }); - onlyInIvy('unknown element check logs a warning rather than throwing') - .it('should not warn about unknown elements with NO_ERRORS_SCHEMA', () => { + onlyInIvy('unknown element check logs an error message rather than throwing') + .it('should not log an error about unknown elements with NO_ERRORS_SCHEMA', () => { @Component({template: ``}) class MyComp { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); TestBed.configureTestingModule({ declarations: [MyComp], schemas: [NO_ERRORS_SCHEMA], @@ -378,8 +378,8 @@ describe('NgModule', () => { }).not.toThrow(); }); - onlyInIvy('unknown element check logs a warning rather than throwing') - .it('should not warn about unknown elements if element matches a directive', () => { + onlyInIvy('unknown element check logs an error message rather than throwing') + .it('should not log an error about unknown elements if element matches a directive', () => { @Component({ selector: 'custom-el', template: '', @@ -391,7 +391,7 @@ describe('NgModule', () => { class MyComp { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); TestBed.configureTestingModule({declarations: [MyComp, CustomEl]}); const fixture = TestBed.createComponent(MyComp); @@ -420,8 +420,8 @@ describe('NgModule', () => { }).not.toThrow(); }); - onlyInIvy('unknown element check logs a warning rather than throwing') - .it('should not warn for HTML elements inside an SVG foreignObject', () => { + onlyInIvy('unknown element check logs an error message rather than throwing') + .it('should not log an error for HTML elements inside an SVG foreignObject', () => { @Component({ template: ` @@ -438,7 +438,7 @@ describe('NgModule', () => { class MyModule { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); TestBed.configureTestingModule({imports: [MyModule]}); const fixture = TestBed.createComponent(MyComp); diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index 3522819021..78ea7f25c6 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -1632,7 +1632,7 @@ function declareTests(config?: {useJit: boolean}) { }); describe('Property bindings', () => { - modifiedInIvy('Unknown property error throws an error instead of logging a warning') + modifiedInIvy('Unknown property error throws an error instead of logging it') .it('should throw on bindings to unknown properties', () => { TestBed.configureTestingModule({declarations: [MyComp]}); const template = '
'; @@ -1646,20 +1646,20 @@ function declareTests(config?: {useJit: boolean}) { } }); - onlyInIvy('Unknown property warning logged instead of throwing an error') + onlyInIvy('Unknown property logs an error message instead of throwing') .it('should throw on bindings to unknown properties', () => { TestBed.configureTestingModule({declarations: [MyComp]}); const template = '
'; TestBed.overrideComponent(MyComp, {set: {template}}); - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); const fixture = TestBed.createComponent(MyComp); fixture.detectChanges(); expect(spy.calls.mostRecent().args[0]) .toMatch(/Can't bind to 'unknown' since it isn't a known property of 'div'./); }); - modifiedInIvy('Unknown property error thrown instead of a warning') + modifiedInIvy('Unknown property error thrown instead of logging it') .it('should throw on bindings to unknown properties', () => { TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]}); const template = '
{{item}}
'; @@ -1675,12 +1675,12 @@ function declareTests(config?: {useJit: boolean}) { } }); - onlyInIvy('Unknown property logs a warning instead of throwing an error') + onlyInIvy('Unknown property logs an error message instead of throwing it') .it('should throw on bindings to unknown properties', () => { TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]}); const template = '
{{item}}
'; TestBed.overrideComponent(MyComp, {set: {template}}); - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); const fixture = TestBed.createComponent(MyComp); fixture.detectChanges(); expect(spy.calls.mostRecent().args[0]) diff --git a/packages/core/test/linker/ng_module_integration_spec.ts b/packages/core/test/linker/ng_module_integration_spec.ts index 152f42e68c..5acc1b3ae7 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -250,7 +250,7 @@ function declareTests(config?: {useJit: boolean}) { expect(() => createModule(SomeModule)).toThrowError(/Can't bind to 'someUnknownProp'/); }); - onlyInIvy('Unknown property warning logged, instead of throwing an error') + onlyInIvy('Unknown property error logged, instead of throwing') .it('should error on unknown bound properties on custom elements by default', () => { @Component({template: '
'}) class ComponentUsingInvalidProperty { @@ -260,7 +260,7 @@ function declareTests(config?: {useJit: boolean}) { class SomeModule { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); const fixture = createComp(ComponentUsingInvalidProperty, SomeModule); fixture.detectChanges(); expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'someUnknownProp'/); diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts index fb077acac7..d4c9102fd4 100644 --- a/packages/core/test/linker/security_integration_spec.ts +++ b/packages/core/test/linker/security_integration_spec.ts @@ -263,12 +263,12 @@ function declareTests(config?: {useJit: boolean}) { .toThrowError(/Can't bind to 'xlink:href'/); }); - onlyInIvy('Unknown property warning logged instead of throwing an error') + onlyInIvy('Unknown property logs an error message instead of throwing') .it('should escape unsafe SVG attributes', () => { const template = `Text`; TestBed.overrideComponent(SecuredComponent, {set: {template}}); - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); const fixture = TestBed.createComponent(SecuredComponent); fixture.detectChanges(); expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'xlink:href'/); diff --git a/packages/platform-browser/test/testing_public_spec.ts b/packages/platform-browser/test/testing_public_spec.ts index 24608a88bd..58d438d7d0 100644 --- a/packages/platform-browser/test/testing_public_spec.ts +++ b/packages/platform-browser/test/testing_public_spec.ts @@ -937,7 +937,7 @@ Did you run and wait for 'resolveComponentResources()'?` : }); - modifiedInIvy(`Unknown property error thrown instead of logging a warning`) + modifiedInIvy(`Unknown property error thrown instead of logging a message`) .it('should error on unknown bound properties on custom elements by default', () => { @Component({template: ''}) class ComponentUsingInvalidProperty { @@ -956,13 +956,13 @@ Did you run and wait for 'resolveComponentResources()'?` : restoreJasmineIt(); }); - onlyInIvy(`Unknown property warning logged instead of an error`) + onlyInIvy(`Unknown property error logged instead of throwing`) .it('should error on unknown bound properties on custom elements by default', () => { @Component({template: '
'}) class ComponentUsingInvalidProperty { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); withModule({declarations: [ComponentUsingInvalidProperty]}, () => { const fixture = TestBed.createComponent(ComponentUsingInvalidProperty); fixture.detectChanges();