diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 97dca5db87..555a5a1f93 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -532,6 +532,21 @@ export function elementContainerStart( const currentQueries = lView[QUERIES]; if (currentQueries) { currentQueries.addNode(tNode); + lView[QUERIES] = currentQueries.clone(); + } + executeContentQueries(tView, tNode); +} + +function executeContentQueries(tView: TView, tNode: TNode) { + if (isContentQueryHost(tNode)) { + const start = tNode.directiveStart; + const end = tNode.directiveEnd; + for (let i = start; i < end; i++) { + const def = tView.data[i] as DirectiveDef; + if (def.contentQueries) { + def.contentQueries(i); + } + } } } @@ -543,7 +558,7 @@ export function elementContainerEnd(): void { if (getIsParent()) { setIsParent(false); } else { - ngDevMode && assertHasParent(getPreviousOrParentTNode()); + ngDevMode && assertHasParent(previousOrParentTNode); previousOrParentTNode = previousOrParentTNode.parent !; setPreviousOrParentTNode(previousOrParentTNode); } @@ -551,8 +566,7 @@ export function elementContainerEnd(): void { ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.ElementContainer); const currentQueries = lView[QUERIES]; if (currentQueries) { - lView[QUERIES] = - isContentQueryHost(previousOrParentTNode) ? currentQueries.parent : currentQueries; + lView[QUERIES] = currentQueries.parent; } registerPostOrderHooks(tView, previousOrParentTNode); @@ -630,7 +644,9 @@ export function elementStart( const currentQueries = lView[QUERIES]; if (currentQueries) { currentQueries.addNode(tNode); + lView[QUERIES] = currentQueries.clone(); } + executeContentQueries(tView, tNode); } /** @@ -668,17 +684,9 @@ function createDirectivesAndLocals( const previousOrParentTNode = getPreviousOrParentTNode(); if (tView.firstTemplatePass) { ngDevMode && ngDevMode.firstTemplatePass++; - resolveDirectives( tView, lView, findDirectiveMatches(tView, lView, previousOrParentTNode), previousOrParentTNode, localRefs || null); - } else { - // During first template pass, queries are created or cloned when first requested - // using `getOrCreateCurrentQueries`. For subsequent template passes, we clone - // any current LQueries here up-front if the current node hosts a content query. - if (isContentQueryHost(getPreviousOrParentTNode()) && lView[QUERIES]) { - lView[QUERIES] = lView[QUERIES] !.clone(); - } } instantiateAllDirectives(tView, lView, previousOrParentTNode); invokeDirectivesHostBindings(tView, lView, previousOrParentTNode); @@ -1068,8 +1076,7 @@ export function elementEnd(): void { const lView = getLView(); const currentQueries = lView[QUERIES]; if (currentQueries) { - lView[QUERIES] = - isContentQueryHost(previousOrParentTNode) ? currentQueries.parent : currentQueries; + lView[QUERIES] = currentQueries.parent; } registerPostOrderHooks(getLView()[TVIEW], previousOrParentTNode); @@ -1805,8 +1812,8 @@ function postProcessDirective( setInputsFromAttrs(directiveDefIdx, directive, def, previousOrParentTNode); } - if (def.contentQueries) { - def.contentQueries(directiveDefIdx); + if (viewData[TVIEW].firstTemplatePass && def.contentQueries) { + previousOrParentTNode.flags |= TNodeFlags.hasContentQuery; } if (isComponentDef(def)) { @@ -2191,7 +2198,7 @@ function containerInternal( function addTContainerToQueries(lView: LView, tContainerNode: TContainerNode): void { const queries = lView[QUERIES]; if (queries) { - lView[QUERIES] = queries.addNode(tContainerNode); + queries.addNode(tContainerNode); const lContainer = lView[tContainerNode.index]; lContainer[QUERIES] = queries.container(); } diff --git a/packages/core/src/render3/interfaces/query.ts b/packages/core/src/render3/interfaces/query.ts index 9f8df300c2..b13c448b51 100644 --- a/packages/core/src/render3/interfaces/query.ts +++ b/packages/core/src/render3/interfaces/query.ts @@ -35,7 +35,7 @@ export interface LQueries { * Notify `LQueries` that a new `TNode` has been created and needs to be added to query results * if matching query predicate. */ - addNode(tNode: TElementNode|TContainerNode|TElementContainerNode): LQueries|null; + addNode(tNode: TElementNode|TContainerNode|TElementContainerNode): void; /** * Notify `LQueries` that a new LContainer was added to ivy data structures. As a result we need diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 04137b24d6..59e305e41c 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -23,9 +23,8 @@ import {unusedValueExportToPlacateAjd as unused1} from './interfaces/definition' import {unusedValueExportToPlacateAjd as unused2} from './interfaces/injector'; import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, unusedValueExportToPlacateAjd as unused3} from './interfaces/node'; import {LQueries, unusedValueExportToPlacateAjd as unused4} from './interfaces/query'; -import {CONTENT_QUERIES, HEADER_OFFSET, LView, TVIEW} from './interfaces/view'; -import {getCurrentQueryIndex, getIsParent, getLView, getOrCreateCurrentQueries, setCurrentQueryIndex} from './state'; -import {isContentQueryHost} from './util'; +import {CONTENT_QUERIES, HEADER_OFFSET, LView, QUERIES, TVIEW} from './interfaces/view'; +import {getCurrentQueryIndex, getIsParent, getLView, setCurrentQueryIndex} from './state'; import {createElementRef, createTemplateRef} from './view_engine_compatibility'; const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4; @@ -107,7 +106,6 @@ export class LQueries_ implements LQueries { container(): LQueries|null { const shallowResults = copyQueriesToContainer(this.shallow); const deepResults = copyQueriesToContainer(this.deep); - return shallowResults || deepResults ? new LQueries_(this, shallowResults, deepResults) : null; } @@ -123,22 +121,9 @@ export class LQueries_ implements LQueries { insertView(index, this.deep); } - addNode(tNode: TElementNode|TContainerNode|TElementContainerNode): LQueries|null { + addNode(tNode: TElementNode|TContainerNode|TElementContainerNode): void { add(this.deep, tNode); - - if (isContentQueryHost(tNode)) { - add(this.shallow, tNode); - - if (tNode.parent && isContentQueryHost(tNode.parent)) { - // if node has a content query and parent also has a content query - // both queries need to check this node for shallow matches - add(this.parent !.shallow, tNode); - } - return this.parent; - } - - isRootNodeOfQuery(tNode) && add(this.shallow, tNode); - return this; + add(this.shallow, tNode); } removeView(): void { @@ -147,10 +132,6 @@ export class LQueries_ implements LQueries { } } -function isRootNodeOfQuery(tNode: TNode) { - return tNode.parent === null || isContentQueryHost(tNode.parent); -} - function copyQueriesToContainer(query: LQuery| null): LQuery|null { let result: LQuery|null = null; @@ -371,11 +352,12 @@ export function query( // TODO: "read" should be an AbstractType (FW-486) predicate: Type| string[], descend?: boolean, read?: any): QueryList { ngDevMode && assertPreviousIsParent(getIsParent()); + const lView = getLView(); const queryList = new QueryList(); - const queries = getOrCreateCurrentQueries(LQueries_); + const queries = lView[QUERIES] || (lView[QUERIES] = new LQueries_(null, null, null)); (queryList as QueryList_)._valuesTree = []; queries.track(queryList, predicate, descend, read); - storeCleanupWithContext(getLView(), queryList, queryList.destroy); + storeCleanupWithContext(lView, queryList, queryList.destroy); return queryList; } diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 869ce8a2a1..8e4b292b1a 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -10,11 +10,8 @@ import {assertDefined} from '../util/assert'; import {executeHooks} from './hooks'; import {ComponentDef, DirectiveDef} from './interfaces/definition'; -import {TElementNode, TNode, TNodeFlags, TViewNode} from './interfaces/node'; -import {LQueries} from './interfaces/query'; -import {BINDING_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, InitPhaseState, LView, LViewFlags, OpaqueViewState, QUERIES, TVIEW, T_HOST} from './interfaces/view'; -import {isContentQueryHost} from './util'; - +import {TElementNode, TNode, TViewNode} from './interfaces/node'; +import {BINDING_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, InitPhaseState, LView, LViewFlags, OpaqueViewState, TVIEW} from './interfaces/view'; /** @@ -165,28 +162,6 @@ export function setIsParent(value: boolean): void { isParent = value; } -/** - * Query instructions can ask for "current queries" in 2 different cases: - * - when creating view queries (at the root of a component view, before any node is created - in - * this case currentQueries points to view queries) - * - when creating content queries (i.e. this previousOrParentTNode points to a node on which we - * create content queries). - */ -export function getOrCreateCurrentQueries( - QueryType: {new (parent: null, shallow: null, deep: null): LQueries}): LQueries { - const lView = getLView(); - let currentQueries = lView[QUERIES]; - // If this is the first content query on a node, any existing LQueries needs to be cloned. - // In subsequent template passes, the cloning occurs before directive instantiation - // in `createDirectivesAndLocals`. - if (previousOrParentTNode && previousOrParentTNode !== lView[T_HOST] && - !isContentQueryHost(previousOrParentTNode)) { - currentQueries && (currentQueries = lView[QUERIES] = currentQueries.clone()); - previousOrParentTNode.flags |= TNodeFlags.hasContentQuery; - } - - return currentQueries || (lView[QUERIES] = new QueryType(null, null, null)); -} /** Checks whether a given view is in creation mode */ export function isCreationMode(view: LView = lView): boolean { diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index c6373060a6..9cd68b01b9 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -50,9 +50,6 @@ { "name": "HOST" }, - { - "name": "T_HOST" - }, { "name": "INJECTOR" }, @@ -131,6 +128,9 @@ { "name": "TVIEW" }, + { + "name": "T_HOST" + }, { "name": "TriggerComponent" }, @@ -266,6 +266,9 @@ { "name": "enterView" }, + { + "name": "executeContentQueries" + }, { "name": "executeHooks" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 597509676b..c17f77c60f 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -80,9 +80,6 @@ { "name": "HOST" }, - { - "name": "T_HOST" - }, { "name": "INJECTOR" }, @@ -218,6 +215,9 @@ { "name": "TVIEW" }, + { + "name": "T_HOST" + }, { "name": "TemplateRef" }, @@ -572,6 +572,9 @@ { "name": "enterView" }, + { + "name": "executeContentQueries" + }, { "name": "executeHooks" }, diff --git a/packages/core/test/forward_ref_integration_spec.ts b/packages/core/test/forward_ref_integration_spec.ts index e8ae122a84..e83651cb06 100644 --- a/packages/core/test/forward_ref_integration_spec.ts +++ b/packages/core/test/forward_ref_integration_spec.ts @@ -40,8 +40,8 @@ class App { } @Component({ - selector: 'lock', - template: `{{frame.name}}({{lock.name}})`, + selector: 'door', + template: `{{frame.name}}({{lock.name}})`, }) class Door { // TODO(issue/24571): remove '!'. diff --git a/packages/core/test/linker/query_integration_spec.ts b/packages/core/test/linker/query_integration_spec.ts index fcaf05e908..29d72ddb13 100644 --- a/packages/core/test/linker/query_integration_spec.ts +++ b/packages/core/test/linker/query_integration_spec.ts @@ -7,6 +7,7 @@ */ import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, Component, ContentChild, ContentChildren, Directive, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef, asNativeElements} from '@angular/core'; +import {ElementRef} from '@angular/core/src/core'; import {ComponentFixture, TestBed, async} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {fixmeIvy, ivyEnabled, modifiedInIvy, onlyInIvy} from '@angular/private/testing'; @@ -45,6 +46,7 @@ describe('Query API', () => { NeedsContentChildWithRead, NeedsViewChildrenWithRead, NeedsViewChildWithRead, + NeedsContentChildrenShallow, NeedsContentChildTemplateRef, NeedsContentChildTemplateRefApp, NeedsViewContainerWithRead, @@ -65,7 +67,9 @@ describe('Query API', () => { `; const view = createTestCmpAndDetectChanges(MyComp0, template); - expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|'); + // Difference in expected text in ivy comes from the fact that ivy queries don't match host + // nodes of a directive that defines a content query. + expect(asNativeElements(view.debugElement.children)).toHaveText(ivyEnabled ? '3|' : '2|3|'); }); it('should contain all direct child directives in the content dom', () => { @@ -172,7 +176,10 @@ describe('Query API', () => { '
'; const view = createTestCmpAndDetectChanges(MyComp0, template); - expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|4|'); + // Difference in expected text in ivy comes from the fact that ivy queries don't match host + // nodes of a directive that defines a content query. + expect(asNativeElements(view.debugElement.children)) + .toHaveText(ivyEnabled ? '3|4|' : '2|3|4|'); }); it('should contain all directives in the light dom', () => { @@ -181,7 +188,9 @@ describe('Query API', () => { '
'; const view = createTestCmpAndDetectChanges(MyComp0, template); - expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|'); + // Difference in expected text in ivy comes from the fact that ivy queries don't match host + // nodes of a directive that defines a content query. + expect(asNativeElements(view.debugElement.children)).toHaveText(ivyEnabled ? '3|' : '2|3|'); }); it('should reflect dynamically inserted directives', () => { @@ -189,11 +198,15 @@ describe('Query API', () => { '
' + '
'; const view = createTestCmpAndDetectChanges(MyComp0, template); - expect(asNativeElements(view.debugElement.children)).toHaveText('2|'); + // Difference in expected text in ivy comes from the fact that ivy queries don't match host + // nodes of a directive that defines a content query. + expect(asNativeElements(view.debugElement.children)).toHaveText(ivyEnabled ? '' : '2|'); view.componentInstance.shouldShow = true; view.detectChanges(); - expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|'); + // Difference in expected text in ivy comes from the fact that ivy queries don't match host + // nodes of a directive that defines a content query. + expect(asNativeElements(view.debugElement.children)).toHaveText(ivyEnabled ? '3|' : '2|3|'); }); it('should be cleanly destroyed when a query crosses view boundaries', () => { @@ -212,11 +225,17 @@ describe('Query API', () => { '
' + '
'; const view = createTestCmpAndDetectChanges(MyComp0, template); - expect(asNativeElements(view.debugElement.children)).toHaveText('2|1d|2d|3d|'); + // Difference in expected text in ivy comes from the fact that ivy queries don't match host + // nodes of a directive that defines a content query. + expect(asNativeElements(view.debugElement.children)) + .toHaveText(ivyEnabled ? '1d|2d|3d|' : '2|1d|2d|3d|'); view.componentInstance.list = ['3d', '2d']; view.detectChanges(); - expect(asNativeElements(view.debugElement.children)).toHaveText('2|3d|2d|'); + // Difference in expected text in ivy comes from the fact that ivy queries don't match host + // nodes of a directive that defines a content query. + expect(asNativeElements(view.debugElement.children)) + .toHaveText(ivyEnabled ? '3d|2d|' : '2|3d|2d|'); }); it('should throw with descriptive error when query selectors are not present', () => { @@ -286,6 +305,68 @@ describe('Query API', () => { expect(comp.textDirChild.text).toEqual('ca'); }); + it('should contain the first descendant content child for shallow queries', () => { + const template = ` +
+ `; + const view = createTestCmpAndDetectChanges(MyComp0, template); + + const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow); + expect(comp.children.length).toBe(1); + }); + + it('should contain the first descendant content child in an embedded template for shallow queries', + () => { + const template = ` + +
+ + `; + const view = createTestCmpAndDetectChanges(MyComp0, template); + + const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow); + expect(comp.children.length).toBe(1); + }); + + + it('should contain the first descendant content child in an embedded template for shallow queries and additional directive', + () => { + const template = ` + +
+ + `; + const view = createTestCmpAndDetectChanges(MyComp0, template); + + const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow); + expect(comp.children.length).toBe(1); + }); + + it('should contain the first descendant content child in an embedded template for shallow queries and additional directive (star syntax)', + () => { + const template = ` +
+ `; + const view = createTestCmpAndDetectChanges(MyComp0, template); + + const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow); + expect(comp.children.length).toBe(1); + }); + + onlyInIvy( + 'Shallow queries don\'t cross ng-container boundaries in ivy (ng-container is treated as a regular element') + .it('should not cross ng-container boundaries with shallow queries', () => { + const template = ` + +
+ + `; + const view = createTestCmpAndDetectChanges(MyComp0, template); + + const comp = view.debugElement.children[0].injector.get(NeedsContentChildrenShallow); + expect(comp.children.length).toBe(0); + }); + it('should contain the first descendant content child templateRef', () => { const template = '' + ''; @@ -958,6 +1039,12 @@ class NeedsContentChildWithRead { @ContentChild('nonExisting', {read: TextDirective}) nonExistingVar !: TextDirective; } +@Component({selector: 'needs-content-children-shallow', template: ''}) +class NeedsContentChildrenShallow { + @ContentChildren('q', {descendants: false}) + children !: QueryList; +} + @Component({ selector: 'needs-content-child-template-ref', template: '
' diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index d62e3d03f7..9a7c334dc8 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -2385,7 +2385,7 @@ describe('query', () => { `Expected content query results to be available when ngAfterContentChecked was called.`); }); - it('should support content query matches on directive hosts', () => { + it('should not match directive host with content queries', () => { /** *
*
@@ -2398,7 +2398,7 @@ describe('query', () => { const fixture = new ComponentFixture(AppComponent); expect(withContentInstance !.foos.length) - .toBe(1, `Expected content query to match
.`); + .toBe(0, `Expected content query not to match
.`); }); it('should match shallow content queries in views inserted / removed by ngIf', () => { @@ -2629,7 +2629,7 @@ describe('query', () => { const fixture = new ComponentFixture(AppComponent); expect(outInstance !.fooBars.length).toBe(1); - expect(inInstance !.fooBars.length).toBe(2); + expect(inInstance !.fooBars.length).toBe(1); }); it('should support nested shallow content queries across multiple component instances', () => { @@ -2668,7 +2668,11 @@ describe('query', () => { function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { elementStart(0, 'div', ['query', ''], ['out', 'query']); - { element(2, 'div', ['query', ''], ['in', 'query', 'foo', '']); } + { + elementStart(2, 'div', ['query', ''], ['in', 'query', 'foo', '']); + { element(5, 'span', ['id', 'bar'], ['foo', '']); } + elementEnd(); + } elementEnd(); } if (rf & RenderFlags.Update) { @@ -2676,7 +2680,7 @@ describe('query', () => { inInstance = load(3); } }, - 5, 0, [QueryDirective]); + 7, 0, [QueryDirective]); const fixture1 = new ComponentFixture(AppComponent); expect(outInstance !.fooBars.length).toBe(1);