Skip to content

Commit

Permalink
fix(ivy): queries should register matches from top to bottom (angular…
Browse files Browse the repository at this point in the history 
…#28319)

PR Close angular#28319
  • Loading branch information
@marclaval @vetom
marclaval authored and vetom committed Jan 31, 2019
1 parent acfb962 commit 3ecd718
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 72 deletions.
16 changes: 14 additions & 2 deletions packages/core/src/render3/instructions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,11 @@ export function elementContainerStart(
appendChild(native, tNode, lView);
createDirectivesAndLocals(tView, lView, localRefs);
attachPatchData(native, lView);

const currentQueries = lView[QUERIES];
if (currentQueries) {
currentQueries.addNode(tNode);
}
}

/** Mark the end of the <ng-container>. */
Expand All @@ -518,7 +523,8 @@ export function elementContainerEnd(): void {
ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.ElementContainer);
const currentQueries = lView[QUERIES];
if (currentQueries) {
lView[QUERIES] = currentQueries.addNode(previousOrParentTNode as TElementContainerNode);
lView[QUERIES] =
isContentQueryHost(previousOrParentTNode) ? currentQueries.parent : currentQueries;
}

registerPostOrderHooks(tView, previousOrParentTNode);
Expand Down Expand Up @@ -592,6 +598,11 @@ export function elementStart(
if (tNode.stylingTemplate && (tNode.flags & TNodeFlags.hasClassInput) === 0) {
renderInitialStylesAndClasses(native, tNode.stylingTemplate, lView[RENDERER]);
}

const currentQueries = lView[QUERIES];
if (currentQueries) {
currentQueries.addNode(tNode);
}
}

/**
Expand Down Expand Up @@ -1027,7 +1038,8 @@ export function elementEnd(): void {
const lView = getLView();
const currentQueries = lView[QUERIES];
if (currentQueries) {
lView[QUERIES] = currentQueries.addNode(previousOrParentTNode as TElementNode);
lView[QUERIES] =
isContentQueryHost(previousOrParentTNode) ? currentQueries.parent : currentQueries;
}

registerPostOrderHooks(getLView()[TVIEW], previousOrParentTNode);
Expand Down
124 changes: 54 additions & 70 deletions packages/core/test/linker/query_integration_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,8 @@ describe('Query API', () => {
}));

describe('querying by directive type', () => {
fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
.it('should contain all direct child directives in the light dom (constructor)', () => {
const template = `
it('should contain all direct child directives in the light dom (constructor)', () => {
const template = `
<div text="1"></div>
<needs-query text="2">
<div text="3">
Expand All @@ -65,10 +63,10 @@ describe('Query API', () => {
</needs-query>
<div text="4"></div>
`;
const view = createTestCmpAndDetectChanges(MyComp0, template);
const view = createTestCmpAndDetectChanges(MyComp0, template);

expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
});
expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
});

it('should contain all direct child directives in the content dom', () => {
const template = '<needs-content-children #q><div text="foo"></div></needs-content-children>';
Expand Down Expand Up @@ -97,7 +95,7 @@ describe('Query API', () => {
});

fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
'FW-982: For queries, ng-template own directives should be registered before the ones from inside the template')
.it('should contain the first content child when target is on <ng-template> with embedded view (issue #16568)',
() => {
const template =
Expand Down Expand Up @@ -168,43 +166,37 @@ describe('Query API', () => {
expect(q.logs).toEqual([['setter', null], ['check', null]]);
});

fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
.it('should contain all directives in the light dom when descendants flag is used', () => {
const template = '<div text="1"></div>' +
'<needs-query-desc text="2"><div text="3">' +
'<div text="4"></div>' +
'</div></needs-query-desc>' +
'<div text="5"></div>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
it('should contain all directives in the light dom when descendants flag is used', () => {
const template = '<div text="1"></div>' +
'<needs-query-desc text="2"><div text="3">' +
'<div text="4"></div>' +
'</div></needs-query-desc>' +
'<div text="5"></div>';
const view = createTestCmpAndDetectChanges(MyComp0, template);

expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|4|');
});
expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|4|');
});

fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
.it('should contain all directives in the light dom', () => {
const template = '<div text="1"></div>' +
'<needs-query text="2"><div text="3"></div></needs-query>' +
'<div text="4"></div>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
it('should contain all directives in the light dom', () => {
const template = '<div text="1"></div>' +
'<needs-query text="2"><div text="3"></div></needs-query>' +
'<div text="4"></div>';
const view = createTestCmpAndDetectChanges(MyComp0, template);

expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
});
expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
});

fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
.it('should reflect dynamically inserted directives', () => {
const template = '<div text="1"></div>' +
'<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|');
it('should reflect dynamically inserted directives', () => {
const template = '<div text="1"></div>' +
'<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|');

view.componentInstance.shouldShow = true;
view.detectChanges();
expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
});
view.componentInstance.shouldShow = true;
view.detectChanges();
expect(asNativeElements(view.debugElement.children)).toHaveText('2|3|');
});

it('should be cleanly destroyed when a query crosses view boundaries', () => {
const template = '<div text="1"></div>' +
Expand All @@ -217,19 +209,17 @@ describe('Query API', () => {
view.destroy();
});

fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
.it('should reflect moved directives', () => {
const template = '<div text="1"></div>' +
'<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|');
it('should reflect moved directives', () => {
const template = '<div text="1"></div>' +
'<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|');

view.componentInstance.list = ['3d', '2d'];
view.detectChanges();
expect(asNativeElements(view.debugElement.children)).toHaveText('2|3d|2d|');
});
view.componentInstance.list = ['3d', '2d'];
view.detectChanges();
expect(asNativeElements(view.debugElement.children)).toHaveText('2|3d|2d|');
});

it('should throw with descriptive error when query selectors are not present', () => {
TestBed.configureTestingModule({declarations: [MyCompBroken0, HasNullQueryCondition]});
Expand Down Expand Up @@ -472,25 +462,19 @@ describe('Query API', () => {
});

describe('querying in the view', () => {
fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
.it('should contain all the elements in the view with that have the given directive',
() => {
const template =
'<needs-view-query #q><div text="ignoreme"></div></needs-view-query>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
});
it('should contain all the elements in the view with that have the given directive', () => {
const template = '<needs-view-query #q><div text="ignoreme"></div></needs-view-query>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
});

fixmeIvy(
'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy')
.it('should not include directive present on the host element', () => {
const template = '<needs-view-query #q text="self"></needs-view-query>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
});
it('should not include directive present on the host element', () => {
const template = '<needs-view-query #q text="self"></needs-view-query>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
const q: NeedsViewQuery = view.debugElement.children[0].references !['q'];
expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2', '3', '4']);
});

it('should reflect changes in the component', () => {
const template = '<needs-view-query-if #q></needs-view-query-if>';
Expand Down
120 changes: 120 additions & 0 deletions packages/core/test/render3/query_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2774,4 +2774,124 @@ describe('query', () => {
expect(deepInstance !.foos.length).toBe(2);
});
});

describe('order', () => {
class TextDirective {
value !: string;

static ngDirectiveDef = defineDirective({
type: TextDirective,
selectors: [['', 'text', '']],
factory: () => new TextDirective(),
inputs: {value: 'text'}
});
}

it('should register content matches from top to bottom', () => {
let contentQueryDirective: ContentQueryDirective;

class ContentQueryDirective {
// @ContentChildren(TextDirective)
texts !: QueryList<TextDirective>;

static ngComponentDef = defineDirective({
type: ContentQueryDirective,
selectors: [['', 'content-query', '']],
factory: () => contentQueryDirective = new ContentQueryDirective(),
contentQueries:
(dirIndex) => { registerContentQuery(query(TextDirective, true), dirIndex); },
contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => {
let tmp: any;
const instance = load<ContentQueryDirective>(dirIndex);
queryRefresh(tmp = loadQueryList<TextDirective>(queryStartIdx)) &&
(instance.texts = tmp);
}
});
}

const AppComponent = createComponent(
'app-component',
/**
* <div content-query>
* <span text="A"></span>
* <div text="B">
* <span text="C">
* <span text="D"></span>
* </span>
* </div>
* <span text="E"></span>
* </div>
*/
function(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
elementStart(0, 'div', ['content-query']);
{
element(1, 'span', ['text', 'A']);
elementStart(2, 'div', ['text', 'B']);
elementStart(3, 'span', ['text', 'C']);
{ element(4, 'span', ['text', 'D']); }
elementEnd();
elementEnd();
element(5, 'span', ['text', 'E']);
}
elementEnd();
}
},
6, 0, [TextDirective, ContentQueryDirective]);

new ComponentFixture(AppComponent);
expect(contentQueryDirective !.texts.map(item => item.value)).toEqual([
'A', 'B', 'C', 'D', 'E'
]);
});

it('should register view matches from top to bottom', () => {
/**
* <span text="A"></span>
* <div text="B">
* <span text="C">
* <span text="D"></span>
* </span>
* </div>
* <span text="E"></span>
*/
class ViewQueryComponent {
// @ViewChildren(TextDirective)
texts !: QueryList<TextDirective>;

static ngComponentDef = defineComponent({
type: ViewQueryComponent,
selectors: [['view-query']],
factory: () => new ViewQueryComponent(),
template: function(rf: RenderFlags, ctx: ViewQueryComponent) {
if (rf & RenderFlags.Create) {
element(0, 'span', ['text', 'A']);
elementStart(1, 'div', ['text', 'B']);
elementStart(2, 'span', ['text', 'C']);
{ element(3, 'span', ['text', 'D']); }
elementEnd();
elementEnd();
element(4, 'span', ['text', 'E']);
}
},
consts: 5,
vars: 0,
viewQuery: function(rf: RenderFlags, ctx: ViewQueryComponent) {
let tmp: any;
if (rf & RenderFlags.Create) {
viewQuery(TextDirective, true);
}
if (rf & RenderFlags.Update) {
queryRefresh(tmp = loadViewQuery<QueryList<TextDirective>>()) &&
(ctx.texts = tmp as QueryList<TextDirective>);
}
},
directives: [TextDirective]
});
}

const fixture = new ComponentFixture(ViewQueryComponent);
expect(fixture.component.texts.map(item => item.value)).toEqual(['A', 'B', 'C', 'D', 'E']);
});
});
});

0 comments on commit 3ecd718

Please sign in to comment.