diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/change_detection/change_detection_jit_generator.ts index 98f4876496..8d9777a4c3 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.ts @@ -203,7 +203,12 @@ export class ChangeDetectorJITGenerator { } } - return notifications.join("\n"); + var directiveNotifications = notifications.join("\n"); + + return ` + this.dispatcher.notifyOnAllChangesDone(); + ${directiveNotifications} + `; } _genLocalDefinitions(): string { return this._localNames.map((n) => `var ${n};`).join("\n"); } diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.ts b/modules/angular2/src/change_detection/dynamic_change_detector.ts index fde3e8dc53..58a4178ca1 100644 --- a/modules/angular2/src/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/change_detection/dynamic_change_detector.ts @@ -110,6 +110,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { } callOnAllChangesDone() { + this.dispatcher.notifyOnAllChangesDone(); var dirs = this.directiveRecords; for (var i = dirs.length - 1; i >= 0; --i) { var dir = dirs[i]; diff --git a/modules/angular2/src/core/compiler/base_query_list.ts b/modules/angular2/src/core/compiler/base_query_list.ts index 212d0c3414..07729f1385 100644 --- a/modules/angular2/src/core/compiler/base_query_list.ts +++ b/modules/angular2/src/core/compiler/base_query_list.ts @@ -26,7 +26,6 @@ export class BaseQueryList { this._dirty = true; } - // TODO(rado): hook up with change detection after #995. fireCallbacks() { if (this._dirty) { ListWrapper.forEach(this._callbacks, (c) => c()); diff --git a/modules/angular2/src/core/compiler/element_injector.ts b/modules/angular2/src/core/compiler/element_injector.ts index 7bd393d1b4..f3b1d55c38 100644 --- a/modules/angular2/src/core/compiler/element_injector.ts +++ b/modules/angular2/src/core/compiler/element_injector.ts @@ -707,6 +707,15 @@ export class ElementInjector extends TreeNode { } } + onAllChangesDone(): void { + if (isPresent(this._query0) && this._query0.originator === this) + this._query0.list.fireCallbacks(); + if (isPresent(this._query1) && this._query1.originator === this) + this._query1.list.fireCallbacks(); + if (isPresent(this._query2) && this._query2.originator === this) + this._query2.list.fireCallbacks(); + } + hydrate(injector: Injector, host: ElementInjector, preBuiltObjects: PreBuiltObjects): void { var p = this._proto; diff --git a/modules/angular2/src/core/compiler/view.ts b/modules/angular2/src/core/compiler/view.ts index ef1dec150e..d07fe2a9de 100644 --- a/modules/angular2/src/core/compiler/view.ts +++ b/modules/angular2/src/core/compiler/view.ts @@ -111,6 +111,13 @@ export class AppView implements ChangeDispatcher, EventDispatcher { } } + notifyOnAllChangesDone(): void { + var ei = this.elementInjectors; + for (var i = ei.length - 1; i >= 0; i--) { + if (isPresent(ei[i])) ei[i].onAllChangesDone(); + } + } + getDirectiveFor(directive: DirectiveIndex) { var elementInjector = this.elementInjectors[directive.elementIndex]; return elementInjector.getDirectiveAtIndex(directive.directiveIndex); diff --git a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart index 1d5eee2090..28d93df88f 100644 --- a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart @@ -262,11 +262,15 @@ class _CodegenState { /// them. String _getCallOnAllChangesDoneBody() { // NOTE(kegluneq): Order is important! - return _directiveRecords.reversed + var directiveNotifications = _directiveRecords.reversed .where((rec) => rec.callOnAllChangesDone) .map((rec) => '${_genGetDirective(rec.directiveIndex)}.onAllChangesDone();') .join(''); + return ''' + _dispatcher.notifyOnAllChangesDone(); + ${directiveNotifications} + '''; } String _genLocalDefinitions() => diff --git a/modules/angular2/test/change_detection/change_detector_spec.ts b/modules/angular2/test/change_detection/change_detector_spec.ts index a7befbd9d1..36c0ef7fd9 100644 --- a/modules/angular2/test/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/change_detection/change_detector_spec.ts @@ -324,6 +324,12 @@ export function main() { }); }); + it('should notify the dispatcher on all changes done', () => { + var val = _createChangeDetector('name', new Person('bob')); + val.changeDetector.detectChanges(); + expect(val.dispatcher.onAllChangesDoneCalled).toEqual(true); + }); + describe('updating directives', () => { var directive1; var directive2; @@ -975,6 +981,7 @@ class FakeDirectives { class TestDispatcher extends ChangeDispatcher { log: List; loggedValues: List; + onAllChangesDoneCalled: boolean = false; constructor() { super(); @@ -984,6 +991,7 @@ class TestDispatcher extends ChangeDispatcher { clear() { this.log = ListWrapper.create(); this.loggedValues = ListWrapper.create(); + this.onAllChangesDoneCalled = true; } notifyOnBinding(binding, value) { @@ -991,6 +999,8 @@ class TestDispatcher extends ChangeDispatcher { ListWrapper.push(this.loggedValues, value); } + notifyOnAllChangesDone() { this.onAllChangesDoneCalled = true; } + _asString(value) { return (isBlank(value) ? 'null' : value.toString()); } } diff --git a/modules/angular2/test/core/compiler/element_injector_spec.ts b/modules/angular2/test/core/compiler/element_injector_spec.ts index fc098214ff..14cf3996b9 100644 --- a/modules/angular2/test/core/compiler/element_injector_spec.ts +++ b/modules/angular2/test/core/compiler/element_injector_spec.ts @@ -12,6 +12,8 @@ import { beforeEach, SpyObject, proxy, + inject, + AsyncTestCompleter, el, containsRegexp } from 'angular2/test_lib'; @@ -730,7 +732,7 @@ export function main() { expect(d.dependency).toBeAnInstanceOf(SimpleDirective); }); - it("should throw when a depenency cannot be resolved", () => { + it("should throw when a dependency cannot be resolved", () => { expect(() => injector(ListWrapper.concat([NeedsDirectiveFromParent], extraBindings))) .toThrowError(containsRegexp( `No provider for ${stringify(SimpleDirective) }! (${stringify(NeedsDirectiveFromParent) } -> ${stringify(SimpleDirective) })`)); @@ -818,7 +820,7 @@ export function main() { }); describe("lifecycle", () => { - it("should call onDestroy on directives subscribed to this event", function() { + it("should call onDestroy on directives subscribed to this event", () => { var inj = injector(ListWrapper.concat( [DirectiveBinding.createFromType(DirectiveWithDestroy, new dirAnn.Directive({lifecycle: [onDestroy]}))], @@ -828,13 +830,45 @@ export function main() { expect(destroy.onDestroyCounter).toBe(1); }); - it("should work with services", function() { + it("should work with services", () => { var inj = injector(ListWrapper.concat( [DirectiveBinding.createFromType( SimpleDirective, new dirAnn.Directive({hostInjector: [SimpleService]}))], 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.onAllChangesDone(); + })); + + 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.onAllChangesDone(); // this does not notify the query + + query.add(new CountingDirective()); + child.parent.onAllChangesDone(); + })); }); describe("dynamicallyCreateComponent", () => { diff --git a/modules/angular2/test/core/compiler/query_integration_spec.ts b/modules/angular2/test/core/compiler/query_integration_spec.ts index 5d7b4f0012..58c91bbab6 100644 --- a/modules/angular2/test/core/compiler/query_integration_spec.ts +++ b/modules/angular2/test/core/compiler/query_integration_spec.ts @@ -18,6 +18,7 @@ import {QueryList} from 'angular2/core'; import {Query, Component, Directive, View} from 'angular2/annotations'; import {NgIf, NgFor} from 'angular2/angular2'; +import {ListWrapper} from 'angular2/src/facade/collection'; import {BrowserDomAdapter} from 'angular2/src/dom/browser_adapter'; @@ -120,6 +121,55 @@ export function main() { async.done(); }); })); + + + it('should notify query on change', + inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { + var template = '' + + '
' + + '
' + + '
'; + + tb.createView(MyComp, {html: template}) + .then((view) => { + var q = view.rawView.locals.get("q"); + view.detectChanges(); + + q.query.onChange(() => { + expect(q.query.first.text).toEqual("1"); + expect(q.query.last.text).toEqual("2"); + async.done(); + }); + + view.context.shouldShow = true; + view.detectChanges(); + }); + })); + + it("should notify child's query before notifying parent's query", + inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { + var template = '' + + '' + + '
' + + '
' + + '
'; + + tb.createView(MyComp, {html: template}) + .then((view) => { + var q1 = view.rawView.locals.get("q1"); + var q2 = view.rawView.locals.get("q2"); + + var firedQ2 = false; + + q2.query.onChange(() => { firedQ2 = true; }); + q1.query.onChange(() => { + expect(firedQ2).toBe(true); + async.done(); + }); + + view.detectChanges(); + }); + })); }); } diff --git a/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart b/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart index 29e2bb0b09..5e7a8bc166 100644 --- a/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart +++ b/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart @@ -51,7 +51,9 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { _alreadyChecked = true; } - void callOnAllChangesDone() {} + void callOnAllChangesDone() { + _dispatcher.notifyOnAllChangesDone(); + } void hydrate(MyComponent context, locals, directives) { mode = 'ALWAYS_CHECK'; diff --git a/modules/benchmarks/src/change_detection/change_detection_benchmark.ts b/modules/benchmarks/src/change_detection/change_detection_benchmark.ts index 269fce5a07..f0864a65b6 100644 --- a/modules/benchmarks/src/change_detection/change_detection_benchmark.ts +++ b/modules/benchmarks/src/change_detection/change_detection_benchmark.ts @@ -381,4 +381,5 @@ class FakeDirectives { class DummyDispatcher extends ChangeDispatcher { notifyOnBinding(bindingRecord, newValue) { throw "Should not be used"; } + notifyOnAllChangesDone() {} }