Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 21 additions & 27 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -883,14 +883,17 @@ export function elementPropertyInternal<T>(

if (ngDevMode) {
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++;
}

const renderer = loadRendererFn ? loadRendererFn(tNode, lView) : lView[RENDERER];
// It is assumed that the sanitizer is only added when the compiler determines that the
// property
// is risky, so sanitization can be done without further checks.
// property is risky, so sanitization can be done without further checks.
value = sanitizer != null ? (sanitizer(value, tNode.tagName || '', propName) as any) : value;
if (isProceduralRenderer(renderer)) {
renderer.setProperty(element as RElement, propName, value);
Expand All @@ -902,7 +905,7 @@ export function elementPropertyInternal<T>(
// If the node is a container and the property didn't
// match any of the inputs or schemas we should throw.
if (ngDevMode && !matchingSchemas(lView, tNode.tagName)) {
throw createUnknownPropertyError(propName, tNode);
warnAboutUnknownProperty(propName, tNode);
}
}
}
Expand Down Expand Up @@ -940,22 +943,13 @@ export function setNgReflectProperty(
}
}

function validateAgainstUnknownProperties(
hostView: LView, element: RElement | RComment, propName: string, tNode: TNode) {
// If the tag matches any of the schemas we shouldn't throw.
if (matchingSchemas(hostView, tNode.tagName)) {
return;
}

// 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 validateProperty(
hostView: LView, element: RElement | RComment, propName: string, tNode: TNode): boolean {
// The property is considered valid if the element matches the schema, it exists on the element
// or it is synthetic, and we are in a browser context (web worker nodes should be skipped).
return matchingSchemas(hostView, tNode.tagName) || propName in element ||
propName[0] === ANIMATION_PROP_PREFIX || typeof Node !== 'function' ||
!(element instanceof Node);
}

function matchingSchemas(hostView: LView, tagName: string | null): boolean {
Expand All @@ -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.
* @param propName Name of the invalid property.
* @param tNode Node on which we encountered the error.
*/
function createUnknownPropertyError(propName: string, tNode: TNode): Error {
return new Error(
`Template error: Can't bind to '${propName}' since it isn't a known property of '${tNode.tagName}'.`);
* Logs a warning 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(
`Can't bind to '${propName}' since it isn't a known property of '${tNode.tagName}'.`);
}

/**
Expand Down
86 changes: 59 additions & 27 deletions packages/core/test/acceptance/ng_module_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import {CommonModule} from '@angular/common';
import {Component, NO_ERRORS_SCHEMA, NgModule} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {modifiedInIvy, onlyInIvy} from '@angular/private/testing';

describe('NgModule', () => {
@Component({template: 'hello'})
Expand Down Expand Up @@ -76,33 +77,64 @@ describe('NgModule', () => {
});

describe('schemas', () => {
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'/);
});
onlyInIvy('Unknown property warning logged instead of throwing an error')
.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]});

const spy = spyOn(console, 'warn');
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
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', () => {
@Component({
Expand Down
45 changes: 28 additions & 17 deletions packages/core/test/linker/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1630,7 +1630,7 @@ function declareTests(config?: {useJit: boolean}) {
});

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', () => {
TestBed.configureTestingModule({declarations: [MyComp]});
const template = '<div unknown="{{ctxProp}}"></div>';
Expand All @@ -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', () => {
TestBed.configureTestingModule({declarations: [MyComp]});
const template = '<div unknown="{{ctxProp}}"></div>';
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 {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
throw 'Should throw';
} catch (e) {
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', () => {
TestBed.configureTestingModule({imports: [CommonModule], declarations: [MyComp]});
const template = '<div *ngFor="let item in ctxArrProp">{{item}}</div>';
TestBed.overrideComponent(MyComp, {set: {template}});

try {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
throw 'Should throw';
} catch (e) {
expect(e.message).toMatch(
/Can't bind to 'ngForIn' since it isn't a known property of 'div'./);
}
});
onlyInIvy('Unknown property logs a warning instead of throwing an error')
.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}});
const spy = spyOn(console, 'warn');
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
expect(spy.calls.mostRecent().args[0])
.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',
() => {
Expand Down
19 changes: 5 additions & 14 deletions packages/core/test/linker/ng_module_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,6 @@ class SomePipe {
class CompUsingModuleDirectiveAndPipe {
}

class DummyConsole implements Console {
public warnings: string[] = [];

log(message: string) {}
warn(message: string) { this.warnings.push(message); }
}

{
if (ivyEnabled) {
describe('ivy', () => { declareTests(); });
Expand All @@ -112,12 +105,8 @@ function declareTests(config?: {useJit: boolean}) {
describe('NgModule', () => {
let compiler: Compiler;
let injector: Injector;
let console: DummyConsole;

beforeEach(() => {
console = new DummyConsole();
TestBed.configureCompiler({...config, providers: [{provide: Console, useValue: console}]});
});
beforeEach(() => { TestBed.configureCompiler(config || {}); });

beforeEach(inject([Compiler, Injector], (_compiler: Compiler, _injector: Injector) => {
compiler = _compiler;
Expand Down Expand Up @@ -261,7 +250,7 @@ function declareTests(config?: {useJit: boolean}) {
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', () => {
@Component({template: '<some-element [someUnknownProp]="true"></some-element>'})
class ComponentUsingInvalidProperty {
Expand All @@ -271,8 +260,10 @@ function declareTests(config?: {useJit: boolean}) {
class SomeModule {
}

const spy = spyOn(console, 'warn');
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',
Expand Down
6 changes: 4 additions & 2 deletions packages/core/test/linker/security_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,13 +263,15 @@ function declareTests(config?: {useJit: boolean}) {
.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', () => {
const template = `<svg:circle [xlink:href]="ctxProp">Text</svg:circle>`;
TestBed.overrideComponent(SecuredComponent, {set: {template}});

const spy = spyOn(console, 'warn');
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', () => {
Expand Down
24 changes: 8 additions & 16 deletions packages/platform-browser/test/testing_public_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
@Component({template: '<some-element [someUnknownProp]="true"></some-element>'})
class ComponentUsingInvalidProperty {
Expand All @@ -941,26 +941,18 @@ Did you run and wait for 'resolveComponentResources()'?` :
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', () => {
@Component({template: '<some-element [someUnknownProp]="true"></some-element>'})
class ComponentUsingInvalidProperty {
}

const itPromise = patchJasmineIt();

expect(
() => it(
'should fail', withModule(
{declarations: [ComponentUsingInvalidProperty]},
() => {
const fixture =
TestBed.createComponent(ComponentUsingInvalidProperty);
fixture.detectChanges();
})))
.toThrowError(/Can't bind to 'someUnknownProp'/);

restoreJasmineIt();
const spy = spyOn(console, 'warn');
withModule({declarations: [ComponentUsingInvalidProperty]}, () => {
const fixture = TestBed.createComponent(ComponentUsingInvalidProperty);
fixture.detectChanges();
})();
expect(spy.calls.mostRecent().args[0]).toMatch(/Can't bind to 'someUnknownProp'/);
});
});

Expand Down