feat(query): implement query update mechanism based on views.

Instead of working with finer grained element injectors, queries now
iterate through the views as static units of modification of the
application structure. Views already contain element injectors in the
correct depth-first preorder.

This allows us to remove children linked lists on element injectors and a
lot of book keeping that is already present at the view level.

Queries are recalculated using the afterContentChecked and
afterViewChecked hooks, only during init and after a view container has
changed.

BREAKING CHANGE:
ViewQuery no longer supports the descendants flag. It queries the whole
component view by default.

Closes #3973
This commit is contained in:
Rado Kirov
2015-08-18 21:51:28 -07:00
committed by Rado Kirov
parent 9d42b52d2c
commit 5ebeaf7c9b
7 changed files with 249 additions and 594 deletions
@@ -44,12 +44,24 @@ import {TemplateRef} from 'angular2/src/core/compiler/template_ref';
import {ElementRef} from 'angular2/src/core/compiler/element_ref';
import {DynamicChangeDetector, ChangeDetectorRef, Parser, Lexer} from 'angular2/src/core/change_detection/change_detection';
import {QueryList} from 'angular2/src/core/compiler/query_list';
import {AppView, AppViewContainer} from "angular2/src/core/compiler/view";
function createDummyView(detector = null) {
function createDummyView(detector = null): AppView {
var res = new SpyView();
res.prop("changeDetector", detector);
res.prop("elementOffset", 0);
return res;
res.prop("elementInjectors", []);
res.prop("viewContainers", []);
res.prop("ownBindersCount", 0);
return <any> res;
}
function addInj(view, inj) {
var injs: ElementInjector[] = view.elementInjectors;
injs.push(inj);
var containers: AppViewContainer[] = view.viewContainers;
containers.push(null);
view.prop("ownBindersCount", view.ownBindersCount + 1);
}
@Injectable()
@@ -63,7 +75,7 @@ class SomeOtherDirective {}
var _constructionCount = 0;
@Injectable()
class CountingDirective {
count;
count: number;
constructor() {
this.count = _constructionCount;
_constructionCount += 1;
@@ -213,17 +225,8 @@ class DirectiveWithDestroy {
onDestroy() { this.onDestroyCounter++; }
}
class TestNode extends TreeNode<TestNode> {
message: string;
constructor(parent: TestNode, message) {
super(parent);
this.message = message;
}
toString() { return this.message; }
}
export function main() {
var defaultPreBuiltObjects = new PreBuiltObjects(null, <any>createDummyView(), <any>new SpyElementRef(), null);
var defaultPreBuiltObjects = new PreBuiltObjects(null, createDummyView(), <any>new SpyElementRef(), null);
// An injector with more than 10 bindings will switch to the dynamic strategy
var dynamicBindings = [];
@@ -241,15 +244,6 @@ export function main() {
return ProtoElementInjector.create(parent, index, directiveBinding, hasShadowRoot, distance, dirVariableBindings);
}
function humanize(tree: TreeNode<any>, names: any[][]) {
var lookupName = (item) =>
ListWrapper.last(ListWrapper.find(names, (pair) => pair[0] === item));
if (tree.children.length == 0) return lookupName(tree);
var children = tree.children.map(m => humanize(m, names));
return [lookupName(tree), children];
}
function injector(bindings, imperativelyCreatedInjector = null, isComponent: boolean = false,
preBuiltObjects = null, attributes = null, dirVariableBindings = null) {
var proto = createPei(null, 0, bindings, 0, isComponent, dirVariableBindings);
@@ -290,74 +284,19 @@ export function main() {
}
describe('TreeNodes', () => {
var root, firstParent, lastParent, node;
var root, child;
/*
Build a tree of the following shape:
root
- p1
- c1
- c2
- p2
- c3
*/
beforeEach(() => {
root = new TestNode(null, 'root');
var p1 = firstParent = new TestNode(root, 'p1');
var p2 = lastParent = new TestNode(root, 'p2');
node = new TestNode(p1, 'c1');
new TestNode(p1, 'c2');
new TestNode(p2, 'c3');
root = new TreeNode(null);
child = new TreeNode(root);
});
// depth-first pre-order.
function walk(node, f) {
if (isBlank(node)) return f;
f(node);
ListWrapper.forEach(node.children, (n) => walk(n, f));
}
function logWalk(node) {
var log = '';
walk(node, (n) => { log += (log.length != 0 ? ', ' : '') + n.toString(); });
return log;
}
it('should support listing children',
() => { expect(logWalk(root)).toEqual('root, p1, c1, c2, p2, c3'); });
it('should support removing the first child node', () => {
firstParent.remove();
expect(firstParent.parent).toEqual(null);
expect(logWalk(root)).toEqual('root, p2, c3');
});
it('should support removing the last child node', () => {
lastParent.remove();
expect(logWalk(root)).toEqual('root, p1, c1, c2');
});
it('should support moving a node at the end of children', () => {
node.remove();
root.addChild(node);
expect(logWalk(root)).toEqual('root, p1, c2, p2, c3, c1');
});
it('should support moving a node in the beginning of children', () => {
node.remove();
lastParent.addChildAfter(node, null);
expect(logWalk(root)).toEqual('root, p1, c2, p2, c1, c3');
});
it('should support moving a node in the middle of children', () => {
node.remove();
lastParent.addChildAfter(node, firstParent);
expect(logWalk(root)).toEqual('root, p1, c2, c1, p2, c3');
it('should support removing and adding the parent', () => {
expect(child.parent).toEqual(root);
child.remove();
expect(child.parent).toEqual(null);
root.addChild(child);
expect(child.parent).toEqual(root);
});
});
@@ -493,8 +432,9 @@ export function main() {
var c1 = protoChild1.instantiate(p);
var c2 = protoChild2.instantiate(p);
expect(humanize(p, [[p, 'parent'], [c1, 'child1'], [c2, 'child2']]))
.toEqual(["parent", ["child1", "child2"]]);
expect(c1.parent).toEqual(p);
expect(c2.parent).toEqual(p);
expect(isBlank(p.parent)).toBeTruthy();
});
describe("direct parent", () => {
@@ -906,38 +846,6 @@ export function main() {
extraBindings));
inj.dehydrate();
});
it("should notify queries", inject([AsyncTestCompleter], (async) => {
var inj = injector(ListWrapper.concat([NeedsQuery], extraBindings));
var query = inj.get(NeedsQuery).query;
query.add(new CountingDirective()); // this marks the query as dirty
query.onChange(() => async.done());
inj.afterContentChecked();
}));
it("should not notify inherited queries", inject([AsyncTestCompleter], (async) => {
var child = parentChildInjectors(ListWrapper.concat([NeedsQuery], extraBindings), []);
var query = child.parent.get(NeedsQuery).query;
var calledOnChange = false;
query.onChange(() => {
// make sure the callback is called only once
expect(calledOnChange).toEqual(false);
expect(query.length).toEqual(2);
calledOnChange = true;
async.done()
});
query.add(new CountingDirective());
child.afterContentChecked(); // this does not notify the query
query.add(new CountingDirective());
child.parent.afterContentChecked();
}));
});
describe('static attributes', () => {
@@ -987,7 +895,7 @@ export function main() {
it("should inject ChangeDetectorRef of the containing component into directives", () => {
var cd = new DynamicChangeDetector(null, null, 0, [], [], null, [], [], [], null);
var view = <any>createDummyView(cd);
var view = createDummyView(cd);
var binding = DirectiveBinding.createFromType(DirectiveNeedsChangeDetectorRef, new DirectiveMetadata());
var inj = injector(ListWrapper.concat([binding], extraBindings), null, false,
new PreBuiltObjects(null, view, <any>new SpyElementRef(), null));
@@ -1022,11 +930,17 @@ export function main() {
});
describe('queries', () => {
var preBuildObjects = defaultPreBuiltObjects;
beforeEach(() => { _constructionCount = 0; });
var dummyView;
var preBuildObjects;
function expectDirectives(query, type, expectedIndex) {
beforeEach(() => { _constructionCount = 0;
dummyView = createDummyView();
preBuildObjects = new PreBuiltObjects(null, dummyView, <any>new SpyElementRef(), null);
});
function expectDirectives(query: QueryList<any>, type, expectedIndex) {
var currentCount = 0;
expect(query.length).toEqual(expectedIndex.length);
iterateListLike(query, (i) => {
expect(i).toBeAnInstanceOf(type);
expect(i.count).toBe(expectedIndex[currentCount]);
@@ -1047,51 +961,25 @@ export function main() {
], extraBindings), null,
false, preBuildObjects);
addInj(dummyView, inj);
inj.afterContentChecked();
expectDirectives(inj.get(NeedsQuery).query, CountingDirective, [0]);
})
});
it('should contain PreBuiltObjects on the same injector', () => {
var preBuiltObjects = new PreBuiltObjects(null, null, null, new TemplateRef(<any>new SpyElementRef()));
var preBuiltObjects = new PreBuiltObjects(null, dummyView, null, new TemplateRef(<any>new SpyElementRef()));
var inj = injector(ListWrapper.concat([
NeedsTemplateRefQuery
], extraBindings), null,
false, preBuiltObjects);
addInj(dummyView, inj);
inj.afterContentChecked();
expect(inj.get(NeedsTemplateRefQuery).query.first).toBe(preBuiltObjects.templateRef);
});
it('should contain multiple directives from the same injector', () => {
var inj = injector(ListWrapper.concat([
NeedsQuery,
CountingDirective,
FancyCountingDirective,
bind(CountingDirective).toAlias(FancyCountingDirective)
], extraBindings), null,
false, preBuildObjects);
expect(inj.get(NeedsQuery).query.length).toEqual(2);
expect(inj.get(NeedsQuery).query.first).toBeAnInstanceOf(CountingDirective);
expect(inj.get(NeedsQuery).query.last).toBeAnInstanceOf(FancyCountingDirective);
})
it('should contain multiple directives from the same injector after linking', () => {
var inj = parentChildInjectors([], ListWrapper.concat([
NeedsQuery,
CountingDirective,
FancyCountingDirective,
bind(CountingDirective).toAlias(FancyCountingDirective)
], extraBindings));
var parent = inj.parent;
inj.unlink();
inj.link(parent);
expect(inj.get(NeedsQuery).query.length).toEqual(2);
expect(inj.get(NeedsQuery).query.first).toBeAnInstanceOf(CountingDirective);
expect(inj.get(NeedsQuery).query.last).toBeAnInstanceOf(FancyCountingDirective);
})
it('should contain the element when no directives are bound to the var binding', () => {
var dirs = [NeedsQueryByVarBindings];
@@ -1102,7 +990,10 @@ export function main() {
var inj = injector(dirs.concat(extraBindings), null,
false, preBuildObjects, null, dirVariableBindings);
expect(inj.get(NeedsQueryByVarBindings).query.first).toBe(defaultPreBuiltObjects.elementRef);
addInj(dummyView, inj);
inj.afterContentChecked();
expect(inj.get(NeedsQueryByVarBindings).query.first).toBe(preBuildObjects.elementRef);
});
it('should contain directives on the same injector when querying by variable bindings' +
@@ -1117,21 +1008,14 @@ export function main() {
var inj = injector(dirs.concat(extraBindings), null,
false, preBuildObjects, null, dirVariableBindings);
addInj(dummyView, inj);
inj.afterContentChecked();
// NeedsQueryByVarBindings queries "one,two", so SimpleDirective should be before NeedsDirective
expect(inj.get(NeedsQueryByVarBindings).query.first).toBeAnInstanceOf(SimpleDirective);
expect(inj.get(NeedsQueryByVarBindings).query.last).toBeAnInstanceOf(NeedsDirective);
});
// Dart's restriction on static types in (a is A) makes this feature hard to implement.
// Current proposal is to add second parameter the Query constructor to take a
// comparison function to support user-defined definition of matching.
//it('should support super class directives', () => {
// var inj = injector([NeedsQuery, FancyCountingDirective], null, null, preBuildObjects);
//
// expectDirectives(inj.get(NeedsQuery).query, FancyCountingDirective, [0]);
//});
it('should contain directives on the same and a child injector in construction order', () => {
var protoParent = createPei(null, 0, [NeedsQuery, CountingDirective]);
var protoChild =
@@ -1142,90 +1026,12 @@ export function main() {
parent.hydrate(null, null, preBuildObjects);
child.hydrate(null, null, preBuildObjects);
addInj(dummyView, parent);
addInj(dummyView, child);
parent.afterContentChecked();
expectDirectives(parent.get(NeedsQuery).query, CountingDirective, [0, 1]);
});
it('should reflect unlinking an injector', () => {
var protoParent = createPei(null, 0, [NeedsQuery, CountingDirective]);
var protoChild =
createPei(protoParent, 1, ListWrapper.concat([CountingDirective], extraBindings));
var parent = protoParent.instantiate(null);
var child = protoChild.instantiate(parent);
parent.hydrate(null, null, preBuildObjects);
child.hydrate(null, null, preBuildObjects);
child.unlink();
expectDirectives(parent.get(NeedsQuery).query, CountingDirective, [0]);
});
it('should reflect moving an injector as a last child', () => {
var protoParent = createPei(null, 0, [NeedsQuery, CountingDirective]);
var protoChild1 = createPei(protoParent, 1, [CountingDirective]);
var protoChild2 =
createPei(protoParent, 1, ListWrapper.concat([CountingDirective], extraBindings));
var parent = protoParent.instantiate(null);
var child1 = protoChild1.instantiate(parent);
var child2 = protoChild2.instantiate(parent);
parent.hydrate(null, null, preBuildObjects);
child1.hydrate(null, null, preBuildObjects);
child2.hydrate(null, null, preBuildObjects);
child1.unlink();
child1.link(parent);
var queryList = parent.get(NeedsQuery).query;
expectDirectives(queryList, CountingDirective, [0, 2, 1]);
});
it('should reflect moving an injector as a first child', () => {
var protoParent = createPei(null, 0, [NeedsQuery, CountingDirective]);
var protoChild1 = createPei(protoParent, 1, [CountingDirective]);
var protoChild2 =
createPei(protoParent, 1, ListWrapper.concat([CountingDirective], extraBindings));
var parent = protoParent.instantiate(null);
var child1 = protoChild1.instantiate(parent);
var child2 = protoChild2.instantiate(parent);
parent.hydrate(null, null, preBuildObjects);
child1.hydrate(null, null, preBuildObjects);
child2.hydrate(null, null, preBuildObjects);
child2.unlink();
child2.linkAfter(parent, null);
var queryList = parent.get(NeedsQuery).query;
expectDirectives(queryList, CountingDirective, [0, 2, 1]);
});
it('should support two concurrent queries for the same directive', () => {
var protoGrandParent = createPei(null, 0, [NeedsQuery]);
var protoParent = createPei(null, 0, [NeedsQuery]);
var protoChild =
createPei(protoParent, 1, ListWrapper.concat([CountingDirective], extraBindings));
var grandParent = protoGrandParent.instantiate(null);
var parent = protoParent.instantiate(grandParent);
var child = protoChild.instantiate(parent);
grandParent.hydrate(null, null, preBuildObjects);
parent.hydrate(null, null, preBuildObjects);
child.hydrate(null, null, preBuildObjects);
var queryList1 = grandParent.get(NeedsQuery).query;
var queryList2 = parent.get(NeedsQuery).query;
expectDirectives(queryList1, CountingDirective, [0]);
expectDirectives(queryList2, CountingDirective, [0]);
child.unlink();
expectDirectives(queryList1, CountingDirective, []);
expectDirectives(queryList2, CountingDirective, []);
});
});
});
});
@@ -382,30 +382,13 @@ export function main() {
view.detectChanges();
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3"]);
async.done();
});
}));
it('should query descendants in the view when the flag is used',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template = '<needs-view-query-desc #q></needs-view-query-desc>';
tcb.overrideTemplate(MyComp, template)
.createAsync(MyComp)
.then((view) => {
var q: NeedsViewQueryDesc = view.componentViewChildren[0].getLocal("q");
view.detectChanges();
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3", "4"]);
async.done();
});
}));
it('should include directive present on the host element',
it('should not include directive present on the host element',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template = '<needs-view-query #q text="self"></needs-view-query>';
@@ -416,7 +399,7 @@ export function main() {
view.detectChanges();
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3"]);
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3", "4"]);
async.done();
});
@@ -437,6 +420,7 @@ export function main() {
q.show = true;
view.detectChanges();
expect(q.query.length).toBe(1);
expect(q.query.first.text).toEqual("1");
@@ -486,6 +470,29 @@ export function main() {
view.detectChanges();
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "-3", "2", "4"]);
async.done();
});
}));
it('should maintain directives in pre-order depth-first DOM order after dynamic insertion',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template = '<needs-view-query-order-with-p #q></needs-view-query-order-with-p>';
tcb.overrideTemplate(MyComp, template)
.createAsync(MyComp)
.then((view) => {
var q: NeedsViewQueryOrderWithParent = view.componentViewChildren[0].getLocal("q");
view.detectChanges();
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3", "4"]);
q.list = ["-3", "2"];
view.detectChanges();
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "-3", "2", "4"]);
async.done();
@@ -566,9 +573,7 @@ class NeedsQueryByLabel {
@Injectable()
class NeedsViewQueryByLabel {
query: QueryList<any>;
constructor(@ViewQuery("textLabel", {descendants: true}) query: QueryList<any>) {
this.query = query;
}
constructor(@ViewQuery("textLabel") query: QueryList<any>) { this.query = query; }
}
@Component({selector: 'needs-query-by-var-bindings'})
@@ -595,8 +600,8 @@ class NeedsQueryAndProject {
@Component({selector: 'needs-view-query'})
@View({
directives: [TextDirective],
template: '<div text="1"><div text="need descendants"></div></div>' +
'<div text="2"></div><div text="3"></div>'
template: '<div text="1"><div text="2"></div></div>' +
'<div text="3"></div><div text="4"></div>'
})
@Injectable()
class NeedsViewQuery {
@@ -604,20 +609,6 @@ class NeedsViewQuery {
constructor(@ViewQuery(TextDirective) query: QueryList<TextDirective>) { this.query = query; }
}
@Component({selector: 'needs-view-query-desc'})
@View({
directives: [TextDirective],
template: '<div text="1"><div text="2"></div></div>' +
'<div text="3"></div><div text="4"></div>'
})
@Injectable()
class NeedsViewQueryDesc {
query: QueryList<TextDirective>;
constructor(@ViewQuery(TextDirective, {descendants: true}) query: QueryList<TextDirective>) {
this.query = query;
}
}
@Component({selector: 'needs-view-query-if'})
@View({directives: [NgIf, TextDirective], template: '<div *ng-if="show" text="1"></div>'})
@Injectable()
@@ -646,10 +637,24 @@ class NeedsViewQueryNestedIf {
}
}
// TODO(rado): once https://github.com/angular/angular/issues/3438 is resolved
// duplicate the test without InertDirective.
@Component({selector: 'needs-view-query-order'})
@View({
directives: [NgFor, TextDirective, InertDirective],
template: '<div text="1"></div>' +
'<div *ng-for="var i of list" [text]="i"></div>' +
'<div text="4"></div>'
})
@Injectable()
class NeedsViewQueryOrder {
query: QueryList<TextDirective>;
list: string[];
constructor(@ViewQuery(TextDirective) query: QueryList<TextDirective>) {
this.query = query;
this.list = ['2', '3'];
}
}
@Component({selector: 'needs-view-query-order-with-p'})
@View({
directives: [NgFor, TextDirective, InertDirective],
template: '<div dir><div text="1"></div>' +
@@ -657,10 +662,10 @@ class NeedsViewQueryNestedIf {
'<div text="4"></div></div>'
})
@Injectable()
class NeedsViewQueryOrder {
class NeedsViewQueryOrderWithParent {
query: QueryList<TextDirective>;
list: string[];
constructor(@ViewQuery(TextDirective, {descendants: true}) query: QueryList<TextDirective>) {
constructor(@ViewQuery(TextDirective) query: QueryList<TextDirective>) {
this.query = query;
this.list = ['2', '3'];
}
@@ -687,11 +692,11 @@ class NeedsTpl {
NeedsQueryByTwoLabels,
NeedsQueryAndProject,
NeedsViewQuery,
NeedsViewQueryDesc,
NeedsViewQueryIf,
NeedsViewQueryNestedIf,
NeedsViewQueryOrder,
NeedsViewQueryByLabel,
NeedsViewQueryOrderWithParent,
NeedsTpl,
TextDirective,
InertDirective,
@@ -131,24 +131,23 @@ export function main() {
var binders = [];
for (var i = 0; i < numInj; i++) {
binders.push(createEmptyElBinder(i > 0 ? binders[i - 1] : null))
};
}
var contextPv = createHostPv(binders);
contextView = createViewWithChildren(contextPv);
}
it('should link the views rootElementInjectors at the given context', () => {
it('should not modify the rootElementInjectors at the given context view', () => {
createViews();
utils.attachViewInContainer(parentView, 0, contextView, 0, 0, childView);
expect(contextView.rootElementInjectors.length).toEqual(2);
expect(contextView.rootElementInjectors.length).toEqual(1);
});
it('should link the views rootElementInjectors after the elementInjector at the given context',
() => {
createViews(2);
utils.attachViewInContainer(parentView, 0, contextView, 1, 0, childView);
expect(childView.rootElementInjectors[0].spy('linkAfter'))
.toHaveBeenCalledWith(contextView.elementInjectors[0],
contextView.elementInjectors[1]);
expect(childView.rootElementInjectors[0].spy('link'))
.toHaveBeenCalledWith(contextView.elementInjectors[0]);
});
});