honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): properly query root nodes of embedded views(shallow queries) (#28560) 

PR Close #28560
This commit is contained in:
Pawel Kozlowski 2019-02-06 11:56:57 +01:00 committed by Miško Hevery
parent 7115e7c427
commit e9bedc63bb
9 changed files with 150 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
packages/core/src/render3/instructions.ts
View File

@ -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<any>;
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<T>(
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();
}

2
packages/core/src/render3/interfaces/query.ts
View File

@ -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

32
packages/core/src/render3/query.ts
View File

@ -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<any>| null): LQuery<any>|null {
let result: LQuery<any>|null = null;
@ -371,11 +352,12 @@ export function query<T>(
// TODO: "read" should be an AbstractType (FW-486)
predicate: Type<any>| string[], descend?: boolean, read?: any): QueryList<T> {
ngDevMode && assertPreviousIsParent(getIsParent());
const lView = getLView();
const queryList = new QueryList<T>();
const queries = getOrCreateCurrentQueries(LQueries_);
const queries = lView[QUERIES] || (lView[QUERIES] = new LQueries_(null, null, null));
(queryList as QueryList_<T>)._valuesTree = [];
queries.track(queryList, predicate, descend, read);
storeCleanupWithContext(getLView(), queryList, queryList.destroy);
storeCleanupWithContext(lView, queryList, queryList.destroy);
return queryList;
}

29
packages/core/src/render3/state.ts
View File

@ -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 {

9
packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
View File

@ -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"
},

9
packages/core/test/bundling/todo/bundle.golden_symbols.json
View File

@ -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"
},

4
packages/core/test/forward_ref_integration_spec.ts
View File

@ -40,8 +40,8 @@ class App {
}
@Component({
selector: 'lock',
template: `{{frame.name}}(<span *ngFor="let lock of locks">{{lock.name}}</span>)`,
selector: 'door',
template: `{{frame.name}}(<span *ngFor="let lock of locks">{{lock.name}}</span>)`,
})
class Door {
// TODO(issue/24571): remove '!'.

101
packages/core/test/linker/query_integration_spec.ts
View File

@ -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', () => {
'<div text="5"></div>';
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', () => {
'<div text="4"></div>';
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', () => {
'<needs-query text="2"><div *ngIf="shouldShow" [text]="\'3\'"></div></needs-query>' +
'<div text="4"></div>';
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', () => {
'<needs-query text="2"><div *ngFor="let i of list" [text]="i"></div></needs-query>' +
'<div text="4"></div>';
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 = `<needs-content-children-shallow>
<div #q></div>
</needs-content-children-shallow>`;
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 = `<needs-content-children-shallow>
<ng-template [ngIf]="true">
<div #q></div>
</ng-template>
</needs-content-children-shallow>`;
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 = `<needs-content-children-shallow>
<ng-template [ngIf]="true">
<div #q directive-needs-content-child></div>
</ng-template>
</needs-content-children-shallow>`;
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 = `<needs-content-children-shallow>
<div *ngIf="true" #q directive-needs-content-child></div>
</needs-content-children-shallow>`;
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 = `<needs-content-children-shallow>
<ng-container>
<div #q></div>
</ng-container>
</needs-content-children-shallow>`;
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 = '<needs-content-child-template-ref-app>' +
'</needs-content-child-template-ref-app>';
@ -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<ElementRef>;
}
@Component({
selector: 'needs-content-child-template-ref',
template: '<div [ngTemplateOutlet]="templateRef"></div>'

14
packages/core/test/render3/query_spec.ts
View File

@ -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', () => {
/**
* <div with-content #foo>
* </div>
@ -2398,7 +2398,7 @@ describe('query', () => {
const fixture = new ComponentFixture(AppComponent);
expect(withContentInstance !.foos.length)
.toBe(1, `Expected content query to match <div with-content #foo>.`);
.toBe(0, `Expected content query not to match <div with-content #foo>.`);
});
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<QueryDirective>(3);
}
},
5, 0, [QueryDirective]);
7, 0, [QueryDirective]);
const fixture1 = new ComponentFixture(AppComponent);
expect(outInstance !.fooBars.length).toBe(1);