fix(ivy): warn instead of throwing for unknown properties (#32463)

Logs a warning instead of throwing when running into a binding to an unknown property in JIT mode. Since we aren't using a schema for the runtime validation anymore, this allows us to support browsers where properties are unsupported.

PR Close #32463
This commit is contained in:
crisbeto 2019-09-04 21:48:04 +02:00 committed by Matias Niemelä
parent 4c3674f660
commit bc061b78be
6 changed files with 122 additions and 100 deletions

View File

@ -883,14 +883,17 @@ export function elementPropertyInternal<T>(
if (ngDevMode) { if (ngDevMode) {
validateAgainstEventProperties(propName); validateAgainstEventProperties(propName);
validateAgainstUnknownProperties(lView, element, propName, tNode); if (!validateProperty(lView, element, propName, tNode)) {
// Return here since we only log warnings for unknown properties.
warnAboutUnknownProperty(propName, tNode);
return;
}
ngDevMode.rendererSetProperty++; ngDevMode.rendererSetProperty++;
} }
const renderer = loadRendererFn ? loadRendererFn(tNode, lView) : lView[RENDERER]; const renderer = loadRendererFn ? loadRendererFn(tNode, lView) : lView[RENDERER];
// It is assumed that the sanitizer is only added when the compiler determines that the // It is assumed that the sanitizer is only added when the compiler determines that the
// property // property is risky, so sanitization can be done without further checks.
// is risky, so sanitization can be done without further checks.
value = sanitizer != null ? (sanitizer(value, tNode.tagName || '', propName) as any) : value; value = sanitizer != null ? (sanitizer(value, tNode.tagName || '', propName) as any) : value;
if (isProceduralRenderer(renderer)) { if (isProceduralRenderer(renderer)) {
renderer.setProperty(element as RElement, propName, value); renderer.setProperty(element as RElement, propName, value);
@ -902,7 +905,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(lView, tNode.tagName)) { if (ngDevMode && !matchingSchemas(lView, tNode.tagName)) {
throw createUnknownPropertyError(propName, tNode); warnAboutUnknownProperty(propName, tNode);
} }
} }
} }
@ -940,22 +943,13 @@ export function setNgReflectProperty(
} }
} }
function validateAgainstUnknownProperties( function validateProperty(
hostView: LView, element: RElement | RComment, propName: string, tNode: TNode) { hostView: LView, element: RElement | RComment, propName: string, tNode: TNode): boolean {
// If the tag matches any of the schemas we shouldn't throw. // The property is considered valid if the element matches the schema, it exists on the element
if (matchingSchemas(hostView, tNode.tagName)) { // or it is synthetic, and we are in a browser context (web worker nodes should be skipped).
return; return matchingSchemas(hostView, tNode.tagName) || propName in element ||
} propName[0] === ANIMATION_PROP_PREFIX || typeof Node !== 'function' ||
!(element instanceof Node);
// If prop is not a known property of the HTML element...
if (!(propName in element) &&
// and we are in a browser context... (web worker nodes should be skipped)
typeof Node === 'function' && element instanceof Node &&
// and isn't a synthetic animation property...
propName[0] !== ANIMATION_PROP_PREFIX) {
// ... it is probably a user error and we should throw.
throw createUnknownPropertyError(propName, tNode);
}
} }
function matchingSchemas(hostView: LView, tagName: string | null): boolean { function matchingSchemas(hostView: LView, tagName: string | null): boolean {
@ -975,13 +969,13 @@ function matchingSchemas(hostView: LView, tagName: string | null): boolean {
} }
/** /**
* Creates an error that should be thrown when encountering an unknown property on an element. * Logs a warning 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 error. * @param tNode Node on which we encountered the property.
*/ */
function createUnknownPropertyError(propName: string, tNode: TNode): Error { function warnAboutUnknownProperty(propName: string, tNode: TNode): void {
return new Error( console.warn(
`Template 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}'.`);
} }
/** /**

View File

@ -9,6 +9,7 @@
import {CommonModule} from '@angular/common'; import {CommonModule} from '@angular/common';
import {Component, NO_ERRORS_SCHEMA, NgModule} from '@angular/core'; import {Component, NO_ERRORS_SCHEMA, NgModule} from '@angular/core';
import {TestBed} from '@angular/core/testing'; import {TestBed} from '@angular/core/testing';
import {modifiedInIvy, onlyInIvy} from '@angular/private/testing';
describe('NgModule', () => { describe('NgModule', () => {
@Component({template: 'hello'}) @Component({template: 'hello'})
@ -76,33 +77,64 @@ describe('NgModule', () => {
}); });
describe('schemas', () => { describe('schemas', () => {
it('should throw on unknown props if NO_ERRORS_SCHEMA is absent', () => { onlyInIvy('Unknown property warning logged instead of throwing an error')
@Component({ .it('should throw on unknown props if NO_ERRORS_SCHEMA is absent', () => {
selector: 'my-comp', @Component({
template: ` selector: 'my-comp',
<ng-container *ngIf="condition"> template: `
<div [unknown-prop]="true"></div> <ng-container *ngIf="condition">
</ng-container> <div [unknown-prop]="true"></div>
`, </ng-container>
}) `,
class MyComp { })
condition = true; class MyComp {
} condition = true;
}
@NgModule({ @NgModule({
imports: [CommonModule], imports: [CommonModule],
declarations: [MyComp], declarations: [MyComp],
}) })
class MyModule { class MyModule {
} }
TestBed.configureTestingModule({imports: [MyModule]}); TestBed.configureTestingModule({imports: [MyModule]});
expect(() => { const spy = spyOn(console, 'warn');
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
}).toThrowError(/Can't bind to 'unknown-prop' since it isn't a known property of 'div'/); expect(spy.calls.mostRecent().args[0])
}); .toMatch(/Can't bind to 'unknown-prop' since it isn't a known property of 'div'/);
});
modifiedInIvy('Unknown properties throw an error instead of logging a warning')
.it('should throw on unknown props if NO_ERRORS_SCHEMA is absent', () => {
@Component({
selector: 'my-comp',
template: `
<ng-container *ngIf="condition">
<div [unknown-prop]="true"></div>
</ng-container>
`,
})
class MyComp {
condition = true;
}
@NgModule({
imports: [CommonModule],
declarations: [MyComp],
})
class MyModule {
}
TestBed.configureTestingModule({imports: [MyModule]});
expect(() => {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
}).toThrowError(/Can't bind to 'unknown-prop' since it isn't a known property of 'div'/);
});
it('should not throw on unknown props if NO_ERRORS_SCHEMA is present', () => { it('should not throw on unknown props if NO_ERRORS_SCHEMA is present', () => {
@Component({ @Component({

View File

@ -1630,7 +1630,7 @@ function declareTests(config?: {useJit: boolean}) {
}); });
describe('Property bindings', () => { describe('Property bindings', () => {
modifiedInIvy('Unknown property error thrown during update mode, not creation mode') modifiedInIvy('Unknown property error throws an error instead of logging a warning')
.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>';
@ -1644,35 +1644,46 @@ function declareTests(config?: {useJit: boolean}) {
} }
}); });
onlyInIvy('Unknown property error thrown during update mode, not creation mode') onlyInIvy('Unknown property warning logged instead of throwing an error')
.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 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')
.it('should throw on bindings to unknown properties', () => {
TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]});
const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>';
TestBed.overrideComponent(MyComp, {set: {template}});
try { try {
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
throw 'Should throw'; throw 'Should throw';
} catch (e) { } catch (e) {
expect(e.message).toMatch( expect(e.message).toMatch(
/Template error: Can't bind to 'unknown' since it isn't a known property of 'div'./); /Can't bind to 'ngForIn' since it isn't a known property of 'div'./);
} }
}); });
it('should throw on bindings to unknown properties of containers', () => { onlyInIvy('Unknown property logs a warning instead of throwing an error')
TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]}); .it('should throw on bindings to unknown properties', () => {
const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>'; TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]});
TestBed.overrideComponent(MyComp, {set: {template}}); const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>';
TestBed.overrideComponent(MyComp, {set: {template}});
try { const spy = spyOn(console, 'warn');
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
throw 'Should throw'; expect(spy.calls.mostRecent().args[0])
} catch (e) { .toMatch(/Can't bind to 'ngForIn' since it isn't a known property of 'div'./);
expect(e.message).toMatch( });
/Can't bind to 'ngForIn' since it isn't a known property of 'div'./);
}
});
it('should not throw for property binding to a non-existing property when there is a matching directive property', it('should not throw for property binding to a non-existing property when there is a matching directive property',
() => { () => {

View File

@ -92,13 +92,6 @@ class SomePipe {
class CompUsingModuleDirectiveAndPipe { class CompUsingModuleDirectiveAndPipe {
} }
class DummyConsole implements Console {
public warnings: string[] = [];
log(message: string) {}
warn(message: string) { this.warnings.push(message); }
}
{ {
if (ivyEnabled) { if (ivyEnabled) {
describe('ivy', () => { declareTests(); }); describe('ivy', () => { declareTests(); });
@ -112,12 +105,8 @@ function declareTests(config?: {useJit: boolean}) {
describe('NgModule', () => { describe('NgModule', () => {
let compiler: Compiler; let compiler: Compiler;
let injector: Injector; let injector: Injector;
let console: DummyConsole;
beforeEach(() => { beforeEach(() => { TestBed.configureCompiler(config || {}); });
console = new DummyConsole();
TestBed.configureCompiler({...config, providers: [{provide: Console, useValue: console}]});
});
beforeEach(inject([Compiler, Injector], (_compiler: Compiler, _injector: Injector) => { beforeEach(inject([Compiler, Injector], (_compiler: Compiler, _injector: Injector) => {
compiler = _compiler; compiler = _compiler;
@ -261,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 error thrown during update mode, not creation mode') onlyInIvy('Unknown property warning logged, instead of throwing an error')
.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 {
@ -271,8 +260,10 @@ function declareTests(config?: {useJit: boolean}) {
class SomeModule { class SomeModule {
} }
const spy = spyOn(console, 'warn');
const fixture = createComp(ComponentUsingInvalidProperty, SomeModule); const fixture = createComp(ComponentUsingInvalidProperty, SomeModule);
expect(() => fixture.detectChanges()).toThrowError(/Can't bind to 'someUnknownProp'/); fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'someUnknownProp'/);
}); });
it('should not error on unknown bound properties on custom elements when using the CUSTOM_ELEMENTS_SCHEMA', it('should not error on unknown bound properties on custom elements when using the CUSTOM_ELEMENTS_SCHEMA',

View File

@ -263,13 +263,15 @@ function declareTests(config?: {useJit: boolean}) {
.toThrowError(/Can't bind to 'xlink:href'/); .toThrowError(/Can't bind to 'xlink:href'/);
}); });
onlyInIvy('Unknown property error thrown during update mode, not creation mode') onlyInIvy('Unknown property warning logged instead of throwing an error')
.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 fixture = TestBed.createComponent(SecuredComponent); const fixture = TestBed.createComponent(SecuredComponent);
expect(() => fixture.detectChanges()).toThrowError(/Can't bind to 'xlink:href'/); fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'xlink:href'/);
}); });
it('should escape unsafe HTML values', () => { it('should escape unsafe HTML values', () => {

View File

@ -922,7 +922,7 @@ Did you run and wait for 'resolveComponentResources()'?` :
}); });
modifiedInIvy(`Unknown property error thrown during update mode, not creation mode`) modifiedInIvy(`Unknown property error thrown instead of logging a warning`)
.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 {
@ -941,26 +941,18 @@ Did you run and wait for 'resolveComponentResources()'?` :
restoreJasmineIt(); restoreJasmineIt();
}); });
onlyInIvy(`Unknown property error thrown during update mode, not creation mode`) onlyInIvy(`Unknown property warning logged instead of an error`)
.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 {
} }
const itPromise = patchJasmineIt(); const spy = spyOn(console, 'warn');
withModule({declarations: [ComponentUsingInvalidProperty]}, () => {
expect( const fixture = TestBed.createComponent(ComponentUsingInvalidProperty);
() => it( fixture.detectChanges();
'should fail', withModule( })();
{declarations: [ComponentUsingInvalidProperty]}, expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'someUnknownProp'/);
() => {
const fixture =
TestBed.createComponent(ComponentUsingInvalidProperty);
fixture.detectChanges();
})))
.toThrowError(/Can't bind to 'someUnknownProp'/);
restoreJasmineIt();
}); });
}); });