X Tutup
Skip to content

feat(query): view query is properly updated when dom changes. #3439

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

Closed
rkirov wants to merge 1 commit into from
Closed

feat(query): view query is properly updated when dom changes. #3439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions modules/angular2/src/change_detection/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {CONST} from 'angular2/src/facade/lang';
import {Locals} from './parser/locals';
import {BindingRecord} from './binding_record';
import {DirectiveIndex, DirectiveRecord} from './directive_record';
import {ChangeDetectorRef} from './change_detector_ref';

/**
* Interface used by Angular to control the change detection strategy for an application.
Expand Down Expand Up @@ -50,6 +51,7 @@ export interface ChangeDispatcher {
export interface ChangeDetector {
parent: ChangeDetector;
mode: string;
ref: ChangeDetectorRef;

addChild(cd: ChangeDetector): void;
addShadowDomChild(cd: ChangeDetector): void;
Expand Down
127 changes: 76 additions & 51 deletions modules/angular2/src/core/compiler/element_injector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ class _Context {

export class ElementInjector extends TreeNode<ElementInjector> implements DependencyProvider {
private _host: ElementInjector;
private _preBuiltObjects = null;
private _preBuiltObjects: PreBuiltObjects = null;

// Queries are added during construction or linking with a new parent.
// They are removed only through unlinking.
Expand Down Expand Up @@ -483,19 +483,35 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
this._host = host;
this._preBuiltObjects = preBuiltObjects;

this._reattachInjectors(imperativelyCreatedInjector);
this._strategy.hydrate();

if (isPresent(host)) {
this._addViewQueries(host);
}

this._reattachInjectors(imperativelyCreatedInjector);
this._strategy.hydrate();

this._addDirectivesToQueries();
this._addVarBindingsToQueries();

// TODO(rado): optimize this call, if view queries are not moved around,
// simply appending to the query list is faster than updating.
this._updateViewQueries();

this.hydrated = true;
}

private _updateViewQueries() {
if (isPresent(this._query0) && this._query0.isViewQuery) {
this._query0.update();
}
if (isPresent(this._query1) && this._query1.isViewQuery) {
this._query1.update();
}
if (isPresent(this._query2) && this._query2.isViewQuery) {
this._query2.update();
}
}

private _debugContext(): any {
var p = this._preBuiltObjects;
var index = p.elementRef.boundElementIndex - p.view.elementOffset;
Expand Down Expand Up @@ -656,22 +672,19 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
}

private _addViewQueries(host: ElementInjector): void {
if (isPresent(host._query0) && host._query0.originator == host &&
host._query0.query.isViewQuery)
this._addViewQuery(host._query0);
if (isPresent(host._query1) && host._query1.originator == host &&
host._query1.query.isViewQuery)
this._addViewQuery(host._query1);
if (isPresent(host._query2) && host._query2.originator == host &&
host._query2.query.isViewQuery)
this._addViewQuery(host._query2);
}

private _addViewQuery(queryRef: QueryRef): void {
// TODO(rado): Replace this.parent check with distanceToParent = 1 when
// https://github.com/angular/angular/issues/2707 is fixed.
if (!queryRef.query.descendants && isPresent(this.parent)) return;
this._assignQueryRef(queryRef);
this._addViewQuery(host._query0, host);
this._addViewQuery(host._query1, host);
this._addViewQuery(host._query2, host);
}

private _addViewQuery(queryRef: QueryRef, host: ElementInjector): void {
if (isBlank(queryRef) || !queryRef.isViewQuery || this._hasQuery(queryRef)) return;
if (host._query0.originator == host) {
// TODO(rado): Replace this.parent check with distanceToParent = 1 when
// https://github.com/angular/angular/issues/2707 is fixed.
if (!queryRef.query.descendants && isPresent(this.parent)) return;
this._assignQueryRef(queryRef);
}
}

private _addVarBindingsToQueries(): void {
Expand Down Expand Up @@ -699,6 +712,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend

private _addDirectivesToQuery(queryRef: QueryRef): void {
if (isBlank(queryRef) || queryRef.query.isVarBindingQuery) return;
if (queryRef.isViewQuery && queryRef.originator == this) return;

var matched = [];
this.addDirectivesMatchingQuery(queryRef.query, matched);
Expand Down Expand Up @@ -755,42 +769,37 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
this._addParentQueries();
}

unlink(): void {
var parent = this.parent;
this.remove();
this._removeParentQueries(parent);
}

private _addParentQueries(): void {
if (isBlank(this.parent)) return;
if (isPresent(this.parent._query0) && !this.parent._query0.query.isViewQuery) {
this._addQueryToTree(this.parent._query0);
if (this.hydrated) this.parent._query0.update();
}
if (isPresent(this.parent._query1) && !this.parent._query1.query.isViewQuery) {
this._addQueryToTree(this.parent._query1);
if (this.hydrated) this.parent._query1.update();
}
if (isPresent(this.parent._query2) && !this.parent._query2.query.isViewQuery) {
this._addQueryToTree(this.parent._query2);
if (this.hydrated) this.parent._query2.update();
}
this._addParentQuery(this.parent._query0);
this._addParentQuery(this.parent._query1);
this._addParentQuery(this.parent._query2);
}

unlink(): void {
var queriesToUpdate = [];
if (isPresent(this.parent._query0)) {
this._pruneQueryFromTree(this.parent._query0);
queriesToUpdate.push(this.parent._query0);
}
if (isPresent(this.parent._query1)) {
this._pruneQueryFromTree(this.parent._query1);
queriesToUpdate.push(this.parent._query1);
}
if (isPresent(this.parent._query2)) {
this._pruneQueryFromTree(this.parent._query2);
queriesToUpdate.push(this.parent._query2);
private _addParentQuery(query): void {
if (isPresent(query) && !this._hasQuery(query)) {
this._addQueryToTree(query);
if (this.hydrated) query.update();
}
}

this.remove();
// TODO(rado): update should work on view queries too, however currently it
// is not implemented, so we filter to non-view queries.
var nonViewQueries = ListWrapper.filter(queriesToUpdate, (q) => !q.query.isViewQuery);
ListWrapper.forEach(nonViewQueries, (q) => q.update());
private _removeParentQueries(parent: ElementInjector): void {
this._removeParentQuery(parent._query0);
this._removeParentQuery(parent._query1);
this._removeParentQuery(parent._query2);
}

private _removeParentQuery(query: QueryRef) {
if (isPresent(query)) {
this._pruneQueryFromTree(query);
query.update();
}
}

private _pruneQueryFromTree(query: QueryRef): void {
Expand Down Expand Up @@ -853,6 +862,12 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
getHost(): ElementInjector { return this._host; }

getBoundElementIndex(): number { return this._proto.index; }

getRootViewInjectors(): ElementInjector[] {
var view = this._preBuiltObjects.view;
return view.getNestedView(view.elementOffset + this.getBoundElementIndex())
.rootElementInjectors;
}
}

interface _ElementInjectorStrategy {
Expand Down Expand Up @@ -1134,9 +1149,19 @@ export class QueryRef {
constructor(public query: Query, public list: QueryList<any>,
public originator: ElementInjector) {}

get isViewQuery(): boolean { return this.query.isViewQuery; }

update(): void {
var aggregator = [];
this.visit(this.originator, aggregator);
if (this.query.isViewQuery) {
// intentionally skipping originator for view queries.
var rootViewInjectors = this.originator.getRootViewInjectors();
for (var i = 0; i < rootViewInjectors.length; i++) {
this.visit(rootViewInjectors[i], aggregator);
}
} else {
this.visit(this.originator, aggregator);
}
this.list.reset(aggregator);
}

Expand Down
5 changes: 3 additions & 2 deletions modules/angular2/src/core/compiler/view_manager_utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,14 @@ export class AppViewManagerUtils {
parentView.viewContainers[boundElementIndex] = viewContainer;
}
ListWrapper.insert(viewContainer.views, atIndex, view);
var elementInjector = contextView.elementInjectors[contextBoundElementIndex];

var sibling;
if (atIndex == 0) {
sibling = null;
sibling = elementInjector;
} else {
sibling = ListWrapper.last(viewContainer.views[atIndex - 1].rootElementInjectors);
}
var elementInjector = contextView.elementInjectors[contextBoundElementIndex];
for (var i = view.rootElementInjectors.length - 1; i >= 0; i--) {
if (isPresent(elementInjector.parent)) {
view.rootElementInjectors[i].linkAfter(elementInjector.parent, sibling);
Expand Down
49 changes: 29 additions & 20 deletions modules/angular2/test/core/compiler/query_integration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ export function main() {

view.detectChanges();

expect(q.query.map((d: TextDirective) => d.text)).toEqual(["self", "1", "2", "3"]);
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3"]);

async.done();
});
Expand Down Expand Up @@ -388,26 +388,29 @@ export function main() {
});
}));

/* TODO(rado): fix and reenable.

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 #q text="self"></needs-view-query-order>';
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template = '<needs-view-query-order #q></needs-view-query-order>';

tcb.overrideTemplate(MyComp, template)
.createAsync(MyComp)
.then((view) => {
var q: NeedsViewQueryOrder = view.componentViewChildren[0].getLocal("q");

tcb.overrideTemplate(MyComp, template)
.createAsync(MyComp)
.then((view) => {
var q:NeedsViewQueryOrder = view.componentViewChildren[0].getLocal("q");
view.detectChanges();

view.detectChanges();
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3", "4"]);

q.list = ["-3", "2"];
view.detectChanges();

expect(q.query.length).toBe(4);
expect(q.query.first.text).toEqual("1");
expect(q.query.first.text).toEqual("4");

async.done();
});
}));*/
expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "-3", "2", "4"]);

async.done();
});
}));
});
});
}
Expand Down Expand Up @@ -532,17 +535,23 @@ 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],
template: '<div text="1">' +
'<div *ng-for="var i of [\'2\', \'3\']" [text]="i"></div>' +
'<div text="4">'
directives: [NgFor, TextDirective, InertDirective],
template: '<div dir><div text="1"></div>' +
'<div *ng-for="var i of list" [text]="i"></div>' +
'<div text="4"></div></div>'
})
@Injectable()
class NeedsViewQueryOrder {
query: QueryList<TextDirective>;
constructor(@ViewQuery(TextDirective) query: QueryList<TextDirective>) { this.query = query; }
list: string[];
constructor(@ViewQuery(TextDirective, {descendants: true}) query: QueryList<TextDirective>) {
this.query = query;
this.list = ['2', '3'];
}
}

@Component({selector: 'my-comp'})
Expand Down
3 changes: 2 additions & 1 deletion modules/angular2/test/core/compiler/view_manager_utils_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ export function main() {
createViews(2);
utils.attachViewInContainer(parentView, 0, contextView, 1, 0, childView);
expect(childView.rootElementInjectors[0].spy('linkAfter'))
.toHaveBeenCalledWith(contextView.elementInjectors[0], null);
.toHaveBeenCalledWith(contextView.elementInjectors[0],
contextView.elementInjectors[1]);
});
});

Expand Down
X Tutup