From 4a9f0bebc362eb9aadfc1beced68a41318cccf0f Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Sun, 15 Mar 2020 09:08:57 -0700 Subject: [PATCH] fix(core): prevent unknown property check for AOT-compiled components (#36072) Prior to this commit, the unknown property check was unnecessarily invoked for AOT-compiled components (for these components, the check happens at compile time). This commit updates the code to avoid unknown property verification for AOT-compiled components by checking whether schemas information is present (as a way to detect whether this is JIT or AOT compiled component). Resolves #35945. PR Close #36072 --- .../core/src/render3/instructions/shared.ts | 6 + .../core/test/acceptance/ng_module_spec.ts | 179 +++++++++++------- 2 files changed, 120 insertions(+), 65 deletions(-) diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index deedddd0cc..39455e437e 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -1038,6 +1038,12 @@ export function setNgReflectProperties( function validateProperty( tView: TView, lView: LView, element: RElement|RComment, propName: string, tNode: TNode): boolean { + // If `schemas` is set to `null`, that's an indication that this Component was compiled in AOT + // mode where this check happens at compile time. In JIT mode, `schemas` is always present and + // defined as an array (as an empty array in case `schemas` field is not defined) and we should + // execute the check below. + if (tView.schemas === null) return true; + // 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). if (matchingSchemas(tView, lView, tNode.tagName) || propName in element || diff --git a/packages/core/test/acceptance/ng_module_spec.ts b/packages/core/test/acceptance/ng_module_spec.ts index 6c6d53643a..9714e37b3e 100644 --- a/packages/core/test/acceptance/ng_module_spec.ts +++ b/packages/core/test/acceptance/ng_module_spec.ts @@ -7,7 +7,7 @@ */ import {CommonModule} from '@angular/common'; -import {Component, CUSTOM_ELEMENTS_SCHEMA, Injectable, InjectionToken, NgModule, NgModuleRef, NO_ERRORS_SCHEMA, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineInjector as defineInjector, ɵɵdefineNgModule as defineNgModule, ɵɵelement as element} from '@angular/core'; +import {Component, CUSTOM_ELEMENTS_SCHEMA, Injectable, InjectionToken, NgModule, NgModuleRef, NO_ERRORS_SCHEMA, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineInjector as defineInjector, ɵɵdefineNgModule as defineNgModule, ɵɵelement as element, ɵɵproperty as property} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {modifiedInIvy, onlyInIvy} from '@angular/private/testing'; @@ -247,73 +247,122 @@ describe('NgModule', () => { }).toThrowError(/'custom' is not a known element/); }); - onlyInIvy('test relies on Ivy-specific AOT format') - .it('should not log unknown element warning for AOT-compiled components', () => { - const spy = spyOn(console, 'warn'); - - /* - * @Component({ - * selector: 'comp', - * template: '', - * }) - * class MyComp {} - */ - class MyComp { - static ɵfac = () => new MyComp(); - static ɵcmp = defineComponent({ - type: MyComp, - selectors: [['comp']], - decls: 1, - vars: 0, - template: - function MyComp_Template(rf, ctx) { - if (rf & 1) { - element(0, 'custom-el'); - } - }, - encapsulation: 2 - }); - } - setClassMetadata( - MyComp, [{ - type: Component, - args: [{ - selector: 'comp', - template: '', - }] - }], - null, null); - - /* - * @NgModule({ - * declarations: [MyComp], - * schemas: [NO_ERRORS_SCHEMA], - * }) - * class MyModule {} - */ - class MyModule { - static ɵmod = defineNgModule({type: MyModule}); - static ɵinj = defineInjector({factory: () => new MyModule()}); - } - setClassMetadata( - MyModule, [{ - type: NgModule, - args: [{ - declarations: [MyComp], - schemas: [NO_ERRORS_SCHEMA], - }] - }], - null, null); - - TestBed.configureTestingModule({ - imports: [MyModule], + onlyInIvy('test relies on Ivy-specific AOT format').describe('AOT-compiled components', () => { + function createComponent( + template: (rf: any) => void, vars: number, consts?: (number|string)[][]) { + class Comp { + static ɵfac = () => new Comp(); + static ɵcmp = defineComponent({ + type: Comp, + selectors: [['comp']], + decls: 1, + vars, + consts, + template, + encapsulation: 2 }); + } + setClassMetadata( + Comp, [{ + type: Component, + args: [ + {selector: 'comp', template: '...'}, + ] + }], + null, null); + return Comp; + } - const fixture = TestBed.createComponent(MyComp); - fixture.detectChanges(); - expect(spy).not.toHaveBeenCalled(); + function createNgModule(Comp: any) { + class Module { + static ɵmod = defineNgModule({type: Module}); + static ɵinj = defineInjector({factory: () => new Module()}); + } + setClassMetadata( + Module, [{ + type: NgModule, + args: [{ + declarations: [Comp], + schemas: [NO_ERRORS_SCHEMA], + }] + }], + null, null); + return Module; + } + + it('should not log unknown element warning for AOT-compiled components', () => { + const spy = spyOn(console, 'warn'); + + /* + * @Component({ + * selector: 'comp', + * template: '', + * }) + * class MyComp {} + */ + const MyComp = createComponent((rf: any) => { + if (rf & 1) { + element(0, 'custom-el'); + } + }, 0); + + /* + * @NgModule({ + * declarations: [MyComp], + * schemas: [NO_ERRORS_SCHEMA], + * }) + * class MyModule {} + */ + const MyModule = createNgModule(MyComp); + + TestBed.configureTestingModule({ + imports: [MyModule], }); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + expect(spy).not.toHaveBeenCalled(); + }); + + it('should not log unknown property warning for AOT-compiled components', () => { + const spy = spyOn(console, 'warn'); + + /* + * @Component({ + * selector: 'comp', + * template: '
', + * }) + * class MyComp {} + */ + const MyComp = createComponent((rf: any) => { + if (rf & 1) { + element(0, 'div', 0); + } + if (rf & 2) { + property('foo', true); + } + }, 1, [[3, 'foo']]); + + /* + * @NgModule({ + * declarations: [MyComp], + * schemas: [NO_ERRORS_SCHEMA], + * }) + * class MyModule {} + */ + const MyModule = createNgModule(MyComp); + + TestBed.configureTestingModule({ + imports: [MyModule], + }); + + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + + expect(spy).not.toHaveBeenCalled(); + }); + }); + onlyInIvy('unknown element check logs a warning rather than throwing') .it('should not warn about unknown elements with CUSTOM_ELEMENTS_SCHEMA', () => { @Component({template: ``}) @@ -519,4 +568,4 @@ describe('NgModule', () => { TestBed.createComponent(TestCmp); expect(componentInstance).toBeAnInstanceOf(TestCmp); }); -}); +}); \ No newline at end of file