diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index bcdd57c9aa..0e5ec00b2f 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -883,14 +883,17 @@ export function elementPropertyInternal( 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); @@ -902,7 +905,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(lView, tNode.tagName)) { - throw createUnknownPropertyError(propName, tNode); + warnAboutUnknownProperty(propName, tNode); } } } @@ -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 { @@ -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}'.`); } /** diff --git a/packages/core/test/acceptance/ng_module_spec.ts b/packages/core/test/acceptance/ng_module_spec.ts index b2e0ddb093..4258f09edb 100644 --- a/packages/core/test/acceptance/ng_module_spec.ts +++ b/packages/core/test/acceptance/ng_module_spec.ts @@ -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'}) @@ -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: ` - -
- - `, - }) - class MyComp { - condition = true; - } + 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: ` + +
+ + `, + }) + class MyComp { + condition = true; + } - @NgModule({ - imports: [CommonModule], - declarations: [MyComp], - }) - class MyModule { - } + @NgModule({ + imports: [CommonModule], + declarations: [MyComp], + }) + class MyModule { + } - TestBed.configureTestingModule({imports: [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'/); - }); + 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: ` + +
+ + `, + }) + 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({ diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index 54a461a742..d7841adc08 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -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 = '
'; @@ -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 = '
'; 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 = '
{{item}}
'; + 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 = '
{{item}}
'; - 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 = '
{{item}}
'; + 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', () => { diff --git a/packages/core/test/linker/ng_module_integration_spec.ts b/packages/core/test/linker/ng_module_integration_spec.ts index 73ebe235f4..5430ca3874 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -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(); }); @@ -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; @@ -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: ''}) class ComponentUsingInvalidProperty { @@ -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', diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts index 36065423b1..fb077acac7 100644 --- a/packages/core/test/linker/security_integration_spec.ts +++ b/packages/core/test/linker/security_integration_spec.ts @@ -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 = `Text`; 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', () => { diff --git a/packages/platform-browser/test/testing_public_spec.ts b/packages/platform-browser/test/testing_public_spec.ts index 88564cf419..1487b075d3 100644 --- a/packages/platform-browser/test/testing_public_spec.ts +++ b/packages/platform-browser/test/testing_public_spec.ts @@ -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: ''}) class ComponentUsingInvalidProperty { @@ -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: ''}) 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'/); }); });