X Tutup
Skip to content

feat(query): make QueryList notify on changes via an observable #4395

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
vsavkin wants to merge 1 commit into from
Closed

feat(query): make QueryList notify on changes via an observable #4395

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
65 changes: 39 additions & 26 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 @@ -383,7 +383,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
this._preBuiltObjects = null;
this._strategy.callOnDestroy();
this._strategy.dehydrate();
this._queryStrategy.clearQueryLists();
this._queryStrategy.dehydrate();
}

hydrate(imperativelyCreatedInjector: Injector, host: ElementInjector,
Expand All @@ -392,6 +392,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
this._preBuiltObjects = preBuiltObjects;

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

this.hydrated = true;
Expand Down Expand Up @@ -604,7 +605,8 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
interface _QueryStrategy {
setContentQueriesAsDirty(): void;
setViewQueriesAsDirty(): void;
clearQueryLists(): void;
hydrate(): void;
dehydrate(): void;
updateContentQueries(): void;
updateViewQueries(): void;
findQuery(query: QueryMetadata): QueryRef;
Expand All @@ -613,7 +615,8 @@ interface _QueryStrategy {
class _EmptyQueryStrategy implements _QueryStrategy {
setContentQueriesAsDirty(): void {}
setViewQueriesAsDirty(): void {}
clearQueryLists(): void {}
hydrate(): void {}
dehydrate(): void {}
updateContentQueries(): void {}
updateViewQueries(): void {}
findQuery(query: QueryMetadata): QueryRef {
Expand All @@ -632,9 +635,9 @@ class InlineQueryStrategy implements _QueryStrategy {

constructor(ei: ElementInjector) {
var protoRefs = ei._proto.protoQueryRefs;
if (protoRefs.length > 0) this.query0 = new QueryRef(protoRefs[0], new QueryList<any>(), ei);
if (protoRefs.length > 1) this.query1 = new QueryRef(protoRefs[1], new QueryList<any>(), ei);
if (protoRefs.length > 2) this.query2 = new QueryRef(protoRefs[2], new QueryList<any>(), ei);
if (protoRefs.length > 0) this.query0 = new QueryRef(protoRefs[0], ei);
if (protoRefs.length > 1) this.query1 = new QueryRef(protoRefs[1], ei);
if (protoRefs.length > 2) this.query2 = new QueryRef(protoRefs[2], ei);
}

setContentQueriesAsDirty(): void {
Expand All @@ -649,39 +652,39 @@ class InlineQueryStrategy implements _QueryStrategy {
if (isPresent(this.query2) && this.query2.isViewQuery) this.query2.dirty = true;
}

clearQueryLists(): void {
if (isPresent(this.query0)) this.query0.reset();
if (isPresent(this.query1)) this.query1.reset();
if (isPresent(this.query2)) this.query2.reset();
hydrate(): void {
if (isPresent(this.query0)) this.query0.hydrate();
if (isPresent(this.query1)) this.query1.hydrate();
if (isPresent(this.query2)) this.query2.hydrate();
}

dehydrate(): void {
if (isPresent(this.query0)) this.query0.dehydrate();
if (isPresent(this.query1)) this.query1.dehydrate();
if (isPresent(this.query2)) this.query2.dehydrate();
}

updateContentQueries() {
if (isPresent(this.query0) && !this.query0.isViewQuery) {
this.query0.update();
this.query0.list.fireCallbacks();
}
if (isPresent(this.query1) && !this.query1.isViewQuery) {
this.query1.update();
this.query1.list.fireCallbacks();
}
if (isPresent(this.query2) && !this.query2.isViewQuery) {
this.query2.update();
this.query2.list.fireCallbacks();
}
}

updateViewQueries() {
if (isPresent(this.query0) && this.query0.isViewQuery) {
this.query0.update();
this.query0.list.fireCallbacks();
}
if (isPresent(this.query1) && this.query1.isViewQuery) {
this.query1.update();
this.query1.list.fireCallbacks();
}
if (isPresent(this.query2) && this.query2.isViewQuery) {
this.query2.update();
this.query2.list.fireCallbacks();
}
}

Expand All @@ -703,7 +706,7 @@ class DynamicQueryStrategy implements _QueryStrategy {
queries: QueryRef[];

constructor(ei: ElementInjector) {
this.queries = ei._proto.protoQueryRefs.map(p => new QueryRef(p, new QueryList<any>(), ei));
this.queries = ei._proto.protoQueryRefs.map(p => new QueryRef(p, ei));
}

setContentQueriesAsDirty(): void {
Expand All @@ -720,10 +723,17 @@ class DynamicQueryStrategy implements _QueryStrategy {
}
}

clearQueryLists(): void {
hydrate(): void {
for (var i = 0; i < this.queries.length; ++i) {
var q = this.queries[i];
q.reset();
q.hydrate();
}
}

dehydrate(): void {
for (var i = 0; i < this.queries.length; ++i) {
var q = this.queries[i];
q.dehydrate();
}
}

Expand All @@ -732,7 +742,6 @@ class DynamicQueryStrategy implements _QueryStrategy {
var q = this.queries[i];
if (!q.isViewQuery) {
q.update();
q.list.fireCallbacks();
}
}
}
Expand All @@ -742,7 +751,6 @@ class DynamicQueryStrategy implements _QueryStrategy {
var q = this.queries[i];
if (q.isViewQuery) {
q.update();
q.list.fireCallbacks();
}
}
}
Expand Down Expand Up @@ -972,8 +980,10 @@ export class ProtoQueryRef {
}

export class QueryRef {
constructor(public protoQueryRef: ProtoQueryRef, public list: QueryList<any>,
private originator: ElementInjector, public dirty: boolean = true) {}
public list: QueryList<any>;
public dirty: boolean;

constructor(public protoQueryRef: ProtoQueryRef, private originator: ElementInjector) {}

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

Expand All @@ -991,6 +1001,8 @@ export class QueryRef {
this.protoQueryRef.setter(dir, this.list);
}
}

this.list.notifyOnChanges();
}

private _update(): void {
Expand Down Expand Up @@ -1073,9 +1085,10 @@ export class QueryRef {
inj.addDirectivesMatchingQuery(this.protoQueryRef.query, aggregator);
}

reset(): void {
this.list.reset([]);
this.list.removeAllCallbacks();
dehydrate(): void { this.list = null; }

hydrate(): void {
this.list = new QueryList<any>();
this.dirty = true;
}
}
39 changes: 10 additions & 29 deletions modules/angular2/src/core/compiler/query_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,19 @@
library angular2.src.core.compiler.query_list;

import 'dart:collection';
import 'package:angular2/src/core/facade/async.dart';

/**
* See query_list.ts
*/
class QueryList<T> extends Object
with IterableMixin<T> {
List<T> _results = [];
List _callbacks = [];
bool _dirty = false;
EventEmitter _emitter = new EventEmitter();

Iterator<T> get iterator => _results.iterator;

/** @private */
void reset(List<T> newList) {
_results = newList;
_dirty = true;
}

void add(T obj) {
_results.add(obj);
_dirty = true;
}

void onChange(callback) {
_callbacks.add(callback);
}

void removeCallback(callback) {
_callbacks.remove(callback);
}

void removeAllCallbacks() {
this._callbacks = [];
}
Stream<Iterable<T>> get changes => _emitter;

int get length => _results.length;
T get first => _results.first;
Expand All @@ -49,10 +28,12 @@ class QueryList<T> extends Object
}

/** @private */
void fireCallbacks() {
if (_dirty) {
_callbacks.forEach((c) => c());
_dirty = false;
}
void reset(List<T> newList) {
_results = newList;
}

/** @private */
void notifyOnChanges() {
_emitter.add(this);
Copy link
Copy Markdown
Contributor

@rkirov rkirov Sep 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the delivery sync or async? Currently, a lot of users have to manually trigger asynchronous execution see - #4186

If we are changing to asynchonous, add that a manual call to scheduleMicrotask is not needed any longer.

}
}
54 changes: 14 additions & 40 deletions modules/angular2/src/core/compiler/query_list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {ListWrapper, MapWrapper} from 'angular2/src/core/facade/collection';
import {getSymbolIterator} from 'angular2/src/core/facade/lang';
import {Observable, EventEmitter} from 'angular2/src/core/facade/async';


/**
Expand All @@ -12,7 +13,7 @@ import {getSymbolIterator} from 'angular2/src/core/facade/lang';
* javascript `for (var i of items)` loops as well as in Angular templates with
* `*ng-for="#i of myList"`.
*
* Changes can be observed by attaching callbacks.
* Changes can be observed by subscribing to the changes `Observable`.
*
* NOTE: In the future this class will implement an `Observable` interface.
*
Expand All @@ -21,45 +22,16 @@ import {getSymbolIterator} from 'angular2/src/core/facade/lang';
* @Component({...})
* class Container {
* constructor(@Query(Item) items: QueryList<Item>) {
* items.onChange(() => console.log(items.length));
* items.changes.subscribe(_ => console.log(items.length));
* }
* }
* ```
*/
export class QueryList<T> {
protected _results: Array < T >= [];
protected _callbacks: Array < () => void >= [];
protected _dirty: boolean = false;

/** @private */
reset(newList: T[]): void {
this._results = newList;
this._dirty = true;
}

/** @private */
add(obj: T): void {
this._results.push(obj);
this._dirty = true;
}

/**
* registers a callback that is called upon each change.
*/
onChange(callback: () => void): void { this._callbacks.push(callback); }

/**
* removes a given callback.
*/
removeCallback(callback: () => void): void { ListWrapper.remove(this._callbacks, callback); }

/**
* removes all callback that have been attached.
*/
removeAllCallbacks(): void { this._callbacks = []; }

toString(): string { return this._results.toString(); }
private _results: Array<T> = [];
private _emitter = new EventEmitter();

get changes(): Observable { return this._emitter; }
get length(): number { return this._results.length; }
get first(): T { return ListWrapper.first(this._results); }
get last(): T { return ListWrapper.last(this._results); }
Expand All @@ -71,11 +43,13 @@ export class QueryList<T> {

[getSymbolIterator()](): any { return this._results[getSymbolIterator()](); }

toString(): string { return this._results.toString(); }

/**
* @private
*/
reset(res: T[]): void { this._results = res; }

/** @private */
fireCallbacks(): void {
if (this._dirty) {
ListWrapper.forEach(this._callbacks, (c) => c());
this._dirty = false;
}
}
notifyOnChanges(): void { this._emitter.next(this); }
}
3 changes: 2 additions & 1 deletion modules/angular2/src/core/forms/directives/select_control_value_accessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {Query, Directive} from 'angular2/src/core/metadata';
import {NgControl} from './ng_control';
import {ControlValueAccessor} from './control_value_accessor';
import {isPresent} from 'angular2/src/core/facade/lang';
import {ObservableWrapper} from 'angular2/src/core/facade/async';
import {setProperty} from './shared';

/**
Expand Down Expand Up @@ -81,6 +82,6 @@ export class SelectControlValueAccessor implements ControlValueAccessor {
registerOnTouched(fn: () => any): void { this.onTouched = fn; }

private _updateValueWhenListOfOptionsChanges(query: QueryList<NgSelectOption>) {
query.onChange(() => this.writeValue(this.value));
ObservableWrapper.subscribe(query.changes, (_) => this.writeValue(this.value));
}
}
9 changes: 5 additions & 4 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 @@ -13,6 +13,7 @@ import {
} from 'angular2/test_lib';

import {isPresent} from 'angular2/src/core/facade/lang';
import {ObservableWrapper} from 'angular2/src/core/facade/async';

import {
Component,
Expand Down Expand Up @@ -263,7 +264,7 @@ export function main() {

});

describe("onChange", () => {
describe("changes", () => {
it('should notify query on change',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template = '<needs-query #q>' +
Expand All @@ -277,7 +278,7 @@ export function main() {
var q = view.debugElement.componentViewChildren[0].getLocal("q");
view.detectChanges();

q.query.onChange(() => {
ObservableWrapper.subscribe(q.query.changes, (_) => {
expect(q.query.first.text).toEqual("1");
expect(q.query.last.text).toEqual("2");
async.done();
Expand All @@ -304,8 +305,8 @@ export function main() {

var firedQ2 = false;

q2.query.onChange(() => { firedQ2 = true; });
q1.query.onChange(() => {
ObservableWrapper.subscribe(q2.query.changes, (_) => { firedQ2 = true; });
ObservableWrapper.subscribe(q1.query.changes, (_) => {
expect(firedQ2).toBe(true);
async.done();
});
Expand Down
Loading
X Tutup