Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ivy): queries should register matches from top to bottom #28319

Closed
wants to merge 2 commits into from
Closed

fix(ivy): queries should register matches from top to bottom #28319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63549d3
fix(ivy): queries should register matches from top to bottom
marclaval Jan 23, 2019
24ba878
fixup! fix(ivy): queries should register matches from top to bottom
marclaval Jan 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions packages/core/src/render3/instructions.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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']);
});
});
});