From 9d9d46f52b45a3b24d609939570b4c9fb45b27d7 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 2 Apr 2020 20:58:03 +0200 Subject: [PATCH] fix(core): log error instead of warning for unknown properties and elements (#36399) 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. BREAKING CHANGE: Warnings about unknown elements are now logged as errors. This won't break your app, but it may trip up tools that expect nothing to be logged via `console.error`. Fixes #35699. PR Close #36399 --- .../core/src/render3/instructions/element.ts | 14 +++---- .../core/src/render3/instructions/shared.ts | 10 ++--- .../core/test/acceptance/ng_module_spec.ts | 42 +++++++++---------- 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, 46 insertions(+), 46 deletions(-) diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 2810fa4073..8f3f80af1f 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -37,7 +37,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); @@ -173,7 +173,7 @@ export function ɵɵelement( ɵɵelementEnd(); } -function warnAboutUnknownElement( +function logUnknownElementError( tView: TView, lView: LView, element: RElement, tNode: TNode, hasDirectives: boolean): void { const schemas = tView.schemas; @@ -199,17 +199,17 @@ function warnAboutUnknownElement( !customElements.get(tagName)); if (isUnknown && !matchingSchemas(tView, lView, tagName)) { - let warning = `'${tagName}' is not a known element:\n`; - warning += `1. If '${ + 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 += `2. If '${ + 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 3796cf1fa6..2487d4f42d 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -992,7 +992,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++; @@ -1011,7 +1011,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); } } } @@ -1105,12 +1105,12 @@ export function matchingSchemas(tView: TView, lView: LView, tagName: string|null } /** - * 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 9714e37b3e..87a7fdd628 100644 --- a/packages/core/test/acceptance/ng_module_spec.ts +++ b/packages/core/test/acceptance/ng_module_spec.ts @@ -101,7 +101,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', @@ -124,7 +124,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]) @@ -189,14 +189,14 @@ 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(); @@ -218,14 +218,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(); @@ -363,13 +363,13 @@ describe('NgModule', () => { }); }); - 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], @@ -397,13 +397,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], @@ -431,8 +431,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: '', @@ -444,7 +444,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); @@ -473,8 +473,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: ` @@ -491,7 +491,7 @@ describe('NgModule', () => { class MyModule { } - const spy = spyOn(console, 'warn'); + const spy = spyOn(console, 'error'); TestBed.configureTestingModule({imports: [MyModule]}); const fixture = TestBed.createComponent(MyComp); @@ -568,4 +568,4 @@ describe('NgModule', () => { TestBed.createComponent(TestCmp); expect(componentInstance).toBeAnInstanceOf(TestCmp); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index 679795b629..fb62ced1e5 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -1642,7 +1642,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 = '
'; @@ -1656,20 +1656,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}}
'; @@ -1685,12 +1685,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 e6f2bf694b..96233e0ed7 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -279,7 +279,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 { @@ -289,7 +289,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 ce8b5c5216..a7a4e311d0 100644 --- a/packages/core/test/linker/security_integration_spec.ts +++ b/packages/core/test/linker/security_integration_spec.ts @@ -267,12 +267,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 600e1415f1..2bd0477403 100644 --- a/packages/platform-browser/test/testing_public_spec.ts +++ b/packages/platform-browser/test/testing_public_spec.ts @@ -963,7 +963,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 { @@ -982,13 +982,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();