honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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
This commit is contained in:
crisbeto 2020-03-03 17:42:44 +01:00 committed by atscott
parent ed44073a58
commit 00f3c58bb9
7 changed files with 47 additions and 47 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

@ -36,7 +36,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);
@ -171,7 +171,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;
@ -197,17 +197,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 += message +=
`1. If '${tagName}' is an Angular component, then verify that it is part of this module.\n`; `1. If '${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 += message +=
`2. If '${tagName}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this 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 { } 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

@ -962,7 +962,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++;
@ -982,7 +982,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);
} }
} }
} }
@ -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 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}'.`);
} }

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

@ -97,7 +97,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',
@ -120,7 +120,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])
@ -185,15 +185,15 @@ 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();
@ -215,14 +215,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();
@ -245,8 +245,8 @@ describe('NgModule', () => {
}); });
onlyInIvy('test relies on Ivy-specific AOT format') onlyInIvy('test relies on Ivy-specific AOT format')
.it('should not log unknown element warning for AOT-compiled components', () => { .it('should not log unknown element error for AOT-compiled components', () => {
const spy = spyOn(console, 'warn'); const spy = spyOn(console, 'error');
/* /*
* @Component({ * @Component({
@ -310,13 +310,13 @@ describe('NgModule', () => {
expect(spy).not.toHaveBeenCalled(); expect(spy).not.toHaveBeenCalled();
}); });
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],
@ -344,13 +344,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],
@ -378,8 +378,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: '',
@ -391,7 +391,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);
@ -420,8 +420,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>
@ -438,7 +438,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);

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

@ -1632,7 +1632,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>';
@ -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', () => { .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>';
@ -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', () => { .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

@ -250,7 +250,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 {
@ -260,7 +260,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

@ -263,12 +263,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

@ -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', () => { .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 {
@ -956,13 +956,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();