cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(core): log error instead of warning for unknown properties and elements (#36399)

Browse Source 
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
This commit is contained in:
crisbeto 2020-04-02 20:58:03 +02:00 committed by Alex Rickabaugh
parent d9c4840a9c
commit 9d9d46f52b
7 changed files with 46 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
packages/core/src/render3/instructions/element.ts
View File

@ -37,7 +37,7 @@ function elementStartFirstCreatePass(
const hasDirectives = const hasDirectives =
resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex)); resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex));
ngDevMode && warnAboutUnknownElement(tView, lView, native, tNode, hasDirectives); ngDevMode && logUnknownElementError(tView, lView, native, tNode, hasDirectives);
if (tNode.mergedAttrs !== null) { if (tNode.mergedAttrs !== null) {
computeStaticStyling(tNode, tNode.mergedAttrs); computeStaticStyling(tNode, tNode.mergedAttrs);
@ -173,7 +173,7 @@ export function ɵɵelement(
ɵɵelementEnd(); ɵɵelementEnd();
} }
function warnAboutUnknownElement( function logUnknownElementError(
tView: TView, lView: LView, element: RElement, tNode: TNode, hasDirectives: boolean): void { tView: TView, lView: LView, element: RElement, tNode: TNode, hasDirectives: boolean): void {
const schemas = tView.schemas; const schemas = tView.schemas;
@ -199,17 +199,17 @@ function warnAboutUnknownElement(
!customElements.get(tagName)); !customElements.get(tagName));
if (isUnknown && !matchingSchemas(tView, lView, tagName)) { if (isUnknown && !matchingSchemas(tView, lView, tagName)) {
let warning = `'${tagName}' is not a known element:\n`; let message = `'${tagName}' is not a known element:\n`;
warning += `1. If '${ message += `1. If '${
tagName}' is an Angular component, then verify that it is part of this module.\n`; tagName}' is an Angular component, then verify that it is part of this module.\n`;
if (tagName && tagName.indexOf('-') > -1) { 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.`; tagName}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`;
} else { } else {
warning += message +=
`2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`; `2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`;
} }
console.warn(warning); console.error(message);
} }
} }
} }

10
packages/core/src/render3/instructions/shared.ts
View File

@ -992,7 +992,7 @@ export function elementPropertyInternal<T>(
validateAgainstEventProperties(propName); validateAgainstEventProperties(propName);
if (!validateProperty(tView, lView, element, propName, tNode)) { if (!validateProperty(tView, lView, element, propName, tNode)) {
// Return here since we only log warnings for unknown properties. // Return here since we only log warnings for unknown properties.
warnAboutUnknownProperty(propName, tNode); logUnknownPropertyError(propName, tNode);
return; return;
} }
ngDevMode.rendererSetProperty++; ngDevMode.rendererSetProperty++;
@ -1011,7 +1011,7 @@ export function elementPropertyInternal<T>(
// If the node is a container and the property didn't // If the node is a container and the property didn't
// match any of the inputs or schemas we should throw. // match any of the inputs or schemas we should throw.
if (ngDevMode && !matchingSchemas(tView, lView, tNode.tagName)) { 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 propName Name of the invalid property.
* @param tNode Node on which we encountered the property. * @param tNode Node on which we encountered the property.
*/ */
function warnAboutUnknownProperty(propName: string, tNode: TNode): void { function logUnknownPropertyError(propName: string, tNode: TNode): void {
console.warn( console.error(
`Can't bind to '${propName}' since it isn't a known property of '${tNode.tagName}'.`); `Can't bind to '${propName}' since it isn't a known property of '${tNode.tagName}'.`);
} }

42
packages/core/test/acceptance/ng_module_spec.ts
View File

@ -101,7 +101,7 @@ describe('NgModule', () => {
}); });
describe('schemas', () => { 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', () => { .it('should throw on unknown props if NO_ERRORS_SCHEMA is absent', () => {
@Component({ @Component({
selector: 'my-comp', selector: 'my-comp',
@ -124,7 +124,7 @@ describe('NgModule', () => {
TestBed.configureTestingModule({imports: [MyModule]}); TestBed.configureTestingModule({imports: [MyModule]});
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]) expect(spy.calls.mostRecent().args[0])
@ -189,14 +189,14 @@ describe('NgModule', () => {
}).not.toThrow(); }).not.toThrow();
}); });
onlyInIvy('unknown element check logs a warning rather than throwing') onlyInIvy('unknown element check logs an error message rather than throwing')
.it('should warn about unknown element without CUSTOM_ELEMENTS_SCHEMA for element with dash in tag name', .it('should log an error about unknown element without CUSTOM_ELEMENTS_SCHEMA for element with dash in tag name',
() => { () => {
@Component({template: `<custom-el></custom-el>`}) @Component({template: `<custom-el></custom-el>`})
class MyComp { class MyComp {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
TestBed.configureTestingModule({declarations: [MyComp]}); TestBed.configureTestingModule({declarations: [MyComp]});
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
@ -218,14 +218,14 @@ describe('NgModule', () => {
}).toThrowError(/'custom-el' is not a known element/); }).toThrowError(/'custom-el' is not a known element/);
}); });
onlyInIvy('unknown element check logs a warning rather than throwing') onlyInIvy('unknown element check logs an error message rather than throwing')
.it('should warn about unknown element without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name', .it('should log an error about unknown element without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name',
() => { () => {
@Component({template: `<custom></custom>`}) @Component({template: `<custom></custom>`})
class MyComp { class MyComp {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
TestBed.configureTestingModule({declarations: [MyComp]}); TestBed.configureTestingModule({declarations: [MyComp]});
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
@ -363,13 +363,13 @@ describe('NgModule', () => {
}); });
}); });
onlyInIvy('unknown element check logs a warning rather than throwing') onlyInIvy('unknown element check logs an error message rather than throwing')
.it('should not warn about unknown elements with CUSTOM_ELEMENTS_SCHEMA', () => { .it('should not log an error about unknown elements with CUSTOM_ELEMENTS_SCHEMA', () => {
@Component({template: `<custom-el></custom-el>`}) @Component({template: `<custom-el></custom-el>`})
class MyComp { class MyComp {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
TestBed.configureTestingModule({ TestBed.configureTestingModule({
declarations: [MyComp], declarations: [MyComp],
schemas: [CUSTOM_ELEMENTS_SCHEMA], schemas: [CUSTOM_ELEMENTS_SCHEMA],
@ -397,13 +397,13 @@ describe('NgModule', () => {
}).not.toThrow(); }).not.toThrow();
}); });
onlyInIvy('unknown element check logs a warning rather than throwing') onlyInIvy('unknown element check logs an error message rather than throwing')
.it('should not warn about unknown elements with NO_ERRORS_SCHEMA', () => { .it('should not log an error about unknown elements with NO_ERRORS_SCHEMA', () => {
@Component({template: `<custom-el></custom-el>`}) @Component({template: `<custom-el></custom-el>`})
class MyComp { class MyComp {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
TestBed.configureTestingModule({ TestBed.configureTestingModule({
declarations: [MyComp], declarations: [MyComp],
schemas: [NO_ERRORS_SCHEMA], schemas: [NO_ERRORS_SCHEMA],
@ -431,8 +431,8 @@ describe('NgModule', () => {
}).not.toThrow(); }).not.toThrow();
}); });
onlyInIvy('unknown element check logs a warning rather than throwing') onlyInIvy('unknown element check logs an error message rather than throwing')
.it('should not warn about unknown elements if element matches a directive', () => { .it('should not log an error about unknown elements if element matches a directive', () => {
@Component({ @Component({
selector: 'custom-el', selector: 'custom-el',
template: '', template: '',
@ -444,7 +444,7 @@ describe('NgModule', () => {
class MyComp { class MyComp {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
TestBed.configureTestingModule({declarations: [MyComp, CustomEl]}); TestBed.configureTestingModule({declarations: [MyComp, CustomEl]});
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
@ -473,8 +473,8 @@ describe('NgModule', () => {
}).not.toThrow(); }).not.toThrow();
}); });
onlyInIvy('unknown element check logs a warning rather than throwing') onlyInIvy('unknown element check logs an error message rather than throwing')
.it('should not warn for HTML elements inside an SVG foreignObject', () => { .it('should not log an error for HTML elements inside an SVG foreignObject', () => {
@Component({ @Component({
template: ` template: `
<svg> <svg>
@ -491,7 +491,7 @@ describe('NgModule', () => {
class MyModule { class MyModule {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
TestBed.configureTestingModule({imports: [MyModule]}); TestBed.configureTestingModule({imports: [MyModule]});
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
@ -568,4 +568,4 @@ describe('NgModule', () => {
TestBed.createComponent(TestCmp); TestBed.createComponent(TestCmp);
expect(componentInstance).toBeAnInstanceOf(TestCmp); expect(componentInstance).toBeAnInstanceOf(TestCmp);
}); });
}); });

12
packages/core/test/linker/integration_spec.ts
View File

@ -1642,7 +1642,7 @@ function declareTests(config?: {useJit: boolean}) {
}); });
describe('Property bindings', () => { 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', () => { .it('should throw on bindings to unknown properties', () => {
TestBed.configureTestingModule({declarations: [MyComp]}); TestBed.configureTestingModule({declarations: [MyComp]});
const template = '<div unknown="{{ctxProp}}"></div>'; const template = '<div unknown="{{ctxProp}}"></div>';
@ -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', () => { .it('should throw on bindings to unknown properties', () => {
TestBed.configureTestingModule({declarations: [MyComp]}); TestBed.configureTestingModule({declarations: [MyComp]});
const template = '<div unknown="{{ctxProp}}"></div>'; const template = '<div unknown="{{ctxProp}}"></div>';
TestBed.overrideComponent(MyComp, {set: {template}}); TestBed.overrideComponent(MyComp, {set: {template}});
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]) expect(spy.calls.mostRecent().args[0])
.toMatch(/Can't bind to 'unknown' since it isn't a known property of 'div'./); .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', () => { .it('should throw on bindings to unknown properties', () => {
TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]}); TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]});
const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>'; const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>';
@ -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', () => { .it('should throw on bindings to unknown properties', () => {
TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]}); TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]});
const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>'; const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>';
TestBed.overrideComponent(MyComp, {set: {template}}); TestBed.overrideComponent(MyComp, {set: {template}});
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]) expect(spy.calls.mostRecent().args[0])

4
packages/core/test/linker/ng_module_integration_spec.ts
View File

@ -279,7 +279,7 @@ function declareTests(config?: {useJit: boolean}) {
expect(() => createModule(SomeModule)).toThrowError(/Can't bind to 'someUnknownProp'/); 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', () => { .it('should error on unknown bound properties on custom elements by default', () => {
@Component({template: '<div [someUnknownProp]="true"></div>'}) @Component({template: '<div [someUnknownProp]="true"></div>'})
class ComponentUsingInvalidProperty { class ComponentUsingInvalidProperty {
@ -289,7 +289,7 @@ function declareTests(config?: {useJit: boolean}) {
class SomeModule { class SomeModule {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
const fixture = createComp(ComponentUsingInvalidProperty, SomeModule); const fixture = createComp(ComponentUsingInvalidProperty, SomeModule);
fixture.detectChanges(); fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'someUnknownProp'/); expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'someUnknownProp'/);

4
packages/core/test/linker/security_integration_spec.ts
View File

@ -267,12 +267,12 @@ function declareTests(config?: {useJit: boolean}) {
.toThrowError(/Can't bind to 'xlink:href'/); .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', () => { .it('should escape unsafe SVG attributes', () => {
const template = `<svg:circle [xlink:href]="ctxProp">Text</svg:circle>`; const template = `<svg:circle [xlink:href]="ctxProp">Text</svg:circle>`;
TestBed.overrideComponent(SecuredComponent, {set: {template}}); TestBed.overrideComponent(SecuredComponent, {set: {template}});
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
const fixture = TestBed.createComponent(SecuredComponent); const fixture = TestBed.createComponent(SecuredComponent);
fixture.detectChanges(); fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'xlink:href'/); expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'xlink:href'/);

6
packages/platform-browser/test/testing_public_spec.ts
View File

@ -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', () => { .it('should error on unknown bound properties on custom elements by default', () => {
@Component({template: '<some-element [someUnknownProp]="true"></some-element>'}) @Component({template: '<some-element [someUnknownProp]="true"></some-element>'})
class ComponentUsingInvalidProperty { class ComponentUsingInvalidProperty {
@ -982,13 +982,13 @@ Did you run and wait for 'resolveComponentResources()'?` :
restoreJasmineIt(); 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', () => { .it('should error on unknown bound properties on custom elements by default', () => {
@Component({template: '<div [someUnknownProp]="true"></div>'}) @Component({template: '<div [someUnknownProp]="true"></div>'})
class ComponentUsingInvalidProperty { class ComponentUsingInvalidProperty {
} }
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
withModule({declarations: [ComponentUsingInvalidProperty]}, () => { withModule({declarations: [ComponentUsingInvalidProperty]}, () => {
const fixture = TestBed.createComponent(ComponentUsingInvalidProperty); const fixture = TestBed.createComponent(ComponentUsingInvalidProperty);
fixture.detectChanges(); fixture.detectChanges();