From 3c1a1620e3af9deb659f1dc754bcacf585793dcf Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Mon, 18 Feb 2019 18:18:56 -0800 Subject: [PATCH] fix(ivy): support static ContentChild queries (#28811) This commit adds support for the `static: true` flag in `ContentChild` queries. Prior to this commit, all `ContentChild` queries were resolved after change detection ran. This is a problem for backwards compatibility because View Engine also supported "static" queries which would resolve before change detection. Now if users add a `static: true` option, the query will be resolved in creation mode (before change detection runs). For example: ```ts @ContentChild(TemplateRef, {static: true}) template !: TemplateRef; ``` This feature will come in handy for components that need to create components dynamically. PR Close #28811 --- .../compliance/r3_compiler_compliance_spec.ts | 73 +++++++ .../compiler/src/render3/r3_identifiers.ts | 1 + .../compiler/src/render3/view/compiler.ts | 7 +- .../core/src/core_render3_private_export.ts | 1 + packages/core/src/render3/index.ts | 1 + packages/core/src/render3/instructions.ts | 12 +- packages/core/src/render3/interfaces/view.ts | 8 + packages/core/src/render3/jit/environment.ts | 1 + packages/core/src/render3/query.ts | 24 ++- packages/core/test/acceptance/query_spec.ts | 183 +++++++++++++----- 10 files changed, 261 insertions(+), 50 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index b28c67b354..757b034542 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -1694,6 +1694,79 @@ describe('compiler compliance', () => { expectEmit(source, ContentQueryComponentDefinition, 'Invalid ContentQuery declaration'); }); + it('should support static content queries', () => { + const files = { + app: { + ...directive, + 'content_query.ts': ` + import {Component, ContentChild, NgModule} from '@angular/core'; + import {SomeDirective} from './some.directive'; + + @Component({ + selector: 'content-query-component', + template: \` +
+ \` + }) + export class ContentQueryComponent { + @ContentChild(SomeDirective, {static: true}) someDir !: SomeDirective; + @ContentChild('foo', {static: false}) foo !: ElementRef; + } + + @Component({ + selector: 'my-app', + template: \` + +
+ + \` + }) + export class MyApp { } + + @NgModule({declarations: [SomeDirective, ContentQueryComponent, MyApp]}) + export class MyModule { } + ` + } + }; + + const ContentQueryComponentDefinition = ` + ContentQueryComponent.ngComponentDef = $r3$.ɵdefineComponent({ + type: ContentQueryComponent, + selectors: [["content-query-component"]], + factory: function ContentQueryComponent_Factory(t) { + return new (t || ContentQueryComponent)(); + }, + contentQueries: function ContentQueryComponent_ContentQueries(rf, ctx, dirIndex) { + if (rf & 1) { + $r3$.ɵstaticContentQuery(dirIndex, SomeDirective, true, null); + $r3$.ɵcontentQuery(dirIndex, $ref0$, true, null); + } + if (rf & 2) { + var $tmp$; + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.someDir = $tmp$.first)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.foo = $tmp$.first)); + } + }, + ngContentSelectors: $_c1$, + consts: 2, + vars: 0, + template: function ContentQueryComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵprojectionDef(); + $r3$.ɵelementStart(0, "div"); + $r3$.ɵprojection(1); + $r3$.ɵelementEnd(); + } + }, + encapsulation: 2 + });`; + + const result = compile(files, angularFiles); + const source = result.source; + + expectEmit(source, ContentQueryComponentDefinition, 'Invalid ContentQuery declaration'); + }); + it('should support content queries with read tokens specified', () => { const files = { app: { diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 40e7ee30c7..4ffac6f60c 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -187,6 +187,7 @@ export class Identifiers { static queryRefresh: o.ExternalReference = {name: 'ɵqueryRefresh', moduleName: CORE}; static viewQuery: o.ExternalReference = {name: 'ɵviewQuery', moduleName: CORE}; static staticViewQuery: o.ExternalReference = {name: 'ɵstaticViewQuery', moduleName: CORE}; + static staticContentQuery: o.ExternalReference = {name: 'ɵstaticContentQuery', moduleName: CORE}; static loadViewQuery: o.ExternalReference = {name: 'ɵloadViewQuery', moduleName: CORE}; static contentQuery: o.ExternalReference = {name: 'ɵcontentQuery', moduleName: CORE}; static loadContentQuery: o.ExternalReference = {name: 'ɵloadContentQuery', moduleName: CORE}; diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 25ec289b01..782938fcb9 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -518,9 +518,12 @@ function createContentQueriesFunction( const tempAllocator = temporaryAllocator(updateStatements, TEMPORARY_NAME); for (const query of meta.queries) { - // creation, e.g. r3.contentQuery(dirIndex, somePredicate, true); + // creation, e.g. r3.contentQuery(dirIndex, somePredicate, true, null); const args = [o.variable('dirIndex'), ...prepareQueryParams(query, constantPool) as any]; - createStatements.push(o.importExpr(R3.contentQuery).callFn(args).toStmt()); + + const queryInstruction = query.static ? R3.staticContentQuery : R3.contentQuery; + + createStatements.push(o.importExpr(queryInstruction).callFn(args).toStmt()); // update, e.g. (r3.queryRefresh(tmp = r3.loadContentQuery()) && (ctx.someDir = tmp)); const temporary = tempAllocator(); diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts index fd88d0ba06..3addbc8234 100644 --- a/packages/core/src/core_render3_private_export.ts +++ b/packages/core/src/core_render3_private_export.ts @@ -82,6 +82,7 @@ export { queryRefresh as ɵqueryRefresh, viewQuery as ɵviewQuery, staticViewQuery as ɵstaticViewQuery, + staticContentQuery as ɵstaticContentQuery, loadViewQuery as ɵloadViewQuery, contentQuery as ɵcontentQuery, loadContentQuery as ɵloadContentQuery, diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index e525473d46..3b415f02a4 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -128,6 +128,7 @@ export { loadViewQuery, contentQuery, loadContentQuery, + staticContentQuery } from './query'; export { diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 1a350e7469..76576b8045 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -65,6 +65,8 @@ const enum BindingDirection { */ export function refreshDescendantViews(lView: LView) { const tView = lView[TVIEW]; + const creationMode = isCreationMode(lView); + // This needs to be set before children are processed to support recursive components tView.firstTemplatePass = false; @@ -73,7 +75,7 @@ export function refreshDescendantViews(lView: LView) { // If this is a creation pass, we should not call lifecycle hooks or evaluate bindings. // This will be done in the update pass. - if (!isCreationMode(lView)) { + if (!creationMode) { const checkNoChangesMode = getCheckNoChangesMode(); executeInitHooks(lView, tView, checkNoChangesMode); @@ -90,6 +92,13 @@ export function refreshDescendantViews(lView: LView) { setHostBindings(tView, lView); } + // We resolve content queries specifically marked as `static` in creation mode. Dynamic + // content queries are resolved during change detection (i.e. update mode), after embedded + // views are refreshed (see block above). + if (creationMode && tView.staticContentQueries) { + refreshContentQueries(tView, lView); + } + refreshChildComponents(tView.components); } @@ -785,6 +794,7 @@ export function createTView( expandoInstructions: null, firstTemplatePass: true, staticViewQueries: false, + staticContentQueries: false, initHooks: null, checkHooks: null, contentHooks: null, diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 51c35ee782..589f48a44e 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -384,6 +384,14 @@ export interface TView { */ staticViewQueries: boolean; + /** + * Whether or not there are any static content queries tracked on this view. + * + * We store this so we know whether or not we should do a content query + * refresh after creation mode to collect static query results. + */ + staticContentQueries: boolean; + /** * The index where the viewQueries section of `LView` begins. This section contains * view queries defined for a component/directive. diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts index f48e1f779b..b99c4a112d 100644 --- a/packages/core/src/render3/jit/environment.ts +++ b/packages/core/src/render3/jit/environment.ts @@ -89,6 +89,7 @@ export const angularCoreEnv: {[name: string]: Function} = { 'ɵqueryRefresh': r3.queryRefresh, 'ɵviewQuery': r3.viewQuery, 'ɵstaticViewQuery': r3.staticViewQuery, + 'ɵstaticContentQuery': r3.staticContentQuery, 'ɵloadViewQuery': r3.loadViewQuery, 'ɵcontentQuery': r3.contentQuery, 'ɵloadContentQuery': r3.loadContentQuery, diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index e13cc22692..5ec63a9824 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -442,7 +442,7 @@ export function loadViewQuery(): T { */ export function contentQuery( directiveIndex: number, predicate: Type| string[], descend: boolean, - // TODO: "read" should be an AbstractType (FW-486) + // TODO(FW-486): "read" should be an AbstractType read: any): QueryList { const lView = getLView(); const tView = lView[TVIEW]; @@ -459,6 +459,28 @@ export function contentQuery( return contentQuery; } +/** + * Registers a QueryList, associated with a static content query, for later refresh + * (part of a view refresh). + * + * @param directiveIndex Current directive index + * @param predicate The type for which the query will search + * @param descend Whether or not to descend into children + * @param read What to save in the query + * @returns QueryList + */ +export function staticContentQuery( + directiveIndex: number, predicate: Type| string[], descend: boolean, + // TODO(FW-486): "read" should be an AbstractType + read: any): void { + const queryList = contentQuery(directiveIndex, predicate, descend, read) as QueryList_; + const tView = getLView()[TVIEW]; + queryList._static = true; + if (!tView.staticContentQueries) { + tView.staticContentQueries = true; + } +} + export function loadContentQuery(): QueryList { const lView = getLView(); ngDevMode && diff --git a/packages/core/test/acceptance/query_spec.ts b/packages/core/test/acceptance/query_spec.ts index 58633ed3ab..758b9ebcb9 100644 --- a/packages/core/test/acceptance/query_spec.ts +++ b/packages/core/test/acceptance/query_spec.ts @@ -9,7 +9,7 @@ import {Component, ContentChild, ContentChildren, Directive, ElementRef, Input, QueryList, TemplateRef, Type, ViewChild, ViewChildren} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; -import {fixmeIvy, onlyInIvy} from '@angular/private/testing'; +import {onlyInIvy} from '@angular/private/testing'; describe('query logic', () => { beforeEach(() => { @@ -17,7 +17,7 @@ describe('query logic', () => { declarations: [ AppComp, QueryComp, SimpleCompA, SimpleCompB, StaticViewQueryComp, TextDirective, SubclassStaticViewQueryComp, StaticContentQueryComp, SubclassStaticContentQueryComp, - QueryCompWithChanges + QueryCompWithChanges, StaticContentQueryDir ] }); }); @@ -256,41 +256,37 @@ describe('query logic', () => { expect(comp.contentChildren.length).toBe(2); }); - }); - - fixmeIvy('Must support static content queries in Ivy') - .it('should set static content child queries in creation mode (and just in creation mode)', - () => { - const template = ` + it('should set static content child queries in creation mode (and just in creation mode)', + () => { + const template = `
`; - TestBed.overrideComponent(AppComp, {set: new Component({template})}); - const fixture = TestBed.createComponent(AppComp); - const component = fixture.debugElement.children[0].injector.get(StaticContentQueryComp); + TestBed.overrideComponent(AppComp, {set: new Component({template})}); + const fixture = TestBed.createComponent(AppComp); + const component = fixture.debugElement.children[0].injector.get(StaticContentQueryComp); - // static ContentChild query should be set in creation mode, before CD runs - expect(component.textDir).toBeAnInstanceOf(TextDirective); - expect(component.textDir.text).toEqual(''); - expect(component.setEvents).toEqual(['textDir set']); + // static ContentChild query should be set in creation mode, before CD runs + expect(component.textDir).toBeAnInstanceOf(TextDirective); + expect(component.textDir.text).toEqual(''); + expect(component.setEvents).toEqual(['textDir set']); - // dynamic ContentChild query should not have been resolved yet - expect(component.foo).not.toBeDefined(); + // dynamic ContentChild query should not have been resolved yet + expect(component.foo).not.toBeDefined(); - const span = fixture.nativeElement.querySelector('span'); - (fixture.componentInstance as any).text = 'some text'; - fixture.detectChanges(); + const span = fixture.nativeElement.querySelector('span'); + (fixture.componentInstance as any).text = 'some text'; + fixture.detectChanges(); - expect(component.textDir.text).toEqual('some text'); - expect(component.foo.nativeElement).toBe(span); - expect(component.setEvents).toEqual(['textDir set', 'foo set']); - }); + expect(component.textDir.text).toEqual('some text'); + expect(component.foo.nativeElement).toBe(span); + expect(component.setEvents).toEqual(['textDir set', 'foo set']); + }); - fixmeIvy('Must support static content queries in Ivy') - .it('should support static content child queries inherited from superclasses', () => { - const template = ` + it('should support static content child queries inherited from superclasses', () => { + const template = `
@@ -298,28 +294,100 @@ describe('query logic', () => { `; - TestBed.overrideComponent(AppComp, {set: new Component({template})}); - const fixture = TestBed.createComponent(AppComp); - const component = - fixture.debugElement.children[0].injector.get(SubclassStaticContentQueryComp); - const divs = fixture.nativeElement.querySelectorAll('div'); - const spans = fixture.nativeElement.querySelectorAll('span'); + TestBed.overrideComponent(AppComp, {set: new Component({template})}); + const fixture = TestBed.createComponent(AppComp); + const component = + fixture.debugElement.children[0].injector.get(SubclassStaticContentQueryComp); + const divs = fixture.nativeElement.querySelectorAll('div'); + const spans = fixture.nativeElement.querySelectorAll('span'); - // static ContentChild queries should be set in creation mode, before CD runs - expect(component.textDir).toBeAnInstanceOf(TextDirective); - expect(component.textDir.text).toEqual(''); - expect(component.bar.nativeElement).toEqual(divs[1]); + // static ContentChild queries should be set in creation mode, before CD runs + expect(component.textDir).toBeAnInstanceOf(TextDirective); + expect(component.textDir.text).toEqual(''); + expect(component.bar.nativeElement).toEqual(divs[1]); - // dynamic ContentChild queries should not have been resolved yet - expect(component.foo).not.toBeDefined(); - expect(component.baz).not.toBeDefined(); + // dynamic ContentChild queries should not have been resolved yet + expect(component.foo).not.toBeDefined(); + expect(component.baz).not.toBeDefined(); - (fixture.componentInstance as any).text = 'some text'; - fixture.detectChanges(); - expect(component.textDir.text).toEqual('some text'); - expect(component.foo.nativeElement).toBe(spans[0]); - expect(component.baz.nativeElement).toBe(spans[1]); - }); + (fixture.componentInstance as any).text = 'some text'; + fixture.detectChanges(); + expect(component.textDir.text).toEqual('some text'); + expect(component.foo.nativeElement).toBe(spans[0]); + expect(component.baz.nativeElement).toBe(spans[1]); + }); + + it('should set static content child queries on directives', () => { + const template = ` +
+
+ +
+ `; + TestBed.overrideComponent(AppComp, {set: new Component({template})}); + const fixture = TestBed.createComponent(AppComp); + const component = fixture.debugElement.children[0].injector.get(StaticContentQueryDir); + + // static ContentChild query should be set in creation mode, before CD runs + expect(component.textDir).toBeAnInstanceOf(TextDirective); + expect(component.textDir.text).toEqual(''); + expect(component.setEvents).toEqual(['textDir set']); + + // dynamic ContentChild query should not have been resolved yet + expect(component.foo).not.toBeDefined(); + + const span = fixture.nativeElement.querySelector('span'); + (fixture.componentInstance as any).text = 'some text'; + fixture.detectChanges(); + + expect(component.textDir.text).toEqual('some text'); + expect(component.foo.nativeElement).toBe(span); + expect(component.setEvents).toEqual(['textDir set', 'foo set']); + }); + + it('should support multiple content query components (multiple template passes)', () => { + const template = ` + +
+ + + +
+ + + `; + TestBed.overrideComponent(AppComp, {set: new Component({template})}); + const fixture = TestBed.createComponent(AppComp); + const firstComponent = fixture.debugElement.children[0].injector.get(StaticContentQueryComp); + const secondComponent = fixture.debugElement.children[1].injector.get(StaticContentQueryComp); + + // static ContentChild query should be set in creation mode, before CD runs + expect(firstComponent.textDir).toBeAnInstanceOf(TextDirective); + expect(secondComponent.textDir).toBeAnInstanceOf(TextDirective); + expect(firstComponent.textDir.text).toEqual(''); + expect(secondComponent.textDir.text).toEqual(''); + expect(firstComponent.setEvents).toEqual(['textDir set']); + expect(secondComponent.setEvents).toEqual(['textDir set']); + + // dynamic ContentChild query should not have been resolved yet + expect(firstComponent.foo).not.toBeDefined(); + expect(secondComponent.foo).not.toBeDefined(); + + const spans = fixture.nativeElement.querySelectorAll('span'); + (fixture.componentInstance as any).text = 'some text'; + fixture.detectChanges(); + + expect(firstComponent.textDir.text).toEqual('some text'); + expect(secondComponent.textDir.text).toEqual('some text'); + + expect(firstComponent.foo.nativeElement).toBe(spans[0]); + expect(secondComponent.foo.nativeElement).toBe(spans[1]); + + expect(firstComponent.setEvents).toEqual(['textDir set', 'foo set']); + expect(secondComponent.setEvents).toEqual(['textDir set', 'foo set']); + }); + + }); describe('observable interface', () => { @@ -453,6 +521,29 @@ class StaticContentQueryComp { } } +@Directive({selector: '[staticContentQueryDir]'}) +class StaticContentQueryDir { + private _textDir !: TextDirective; + private _foo !: ElementRef; + setEvents: string[] = []; + + @ContentChild(TextDirective, {static: true}) + get textDir(): TextDirective { return this._textDir; } + + set textDir(value: TextDirective) { + this.setEvents.push('textDir set'); + this._textDir = value; + } + + @ContentChild('foo', {static: false}) + get foo(): ElementRef { return this._foo; } + + set foo(value: ElementRef) { + this.setEvents.push('foo set'); + this._foo = value; + } +} + @Component({selector: 'subclass-static-content-query-comp', template: ``}) class SubclassStaticContentQueryComp extends StaticContentQueryComp { @ContentChild('bar', {static: true})