From d449ea5ca40d8d5904feee2fa92f27e3ffc5616b Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 15 Jul 2015 13:26:02 -0700 Subject: [PATCH] feat(change_detection): added support for ObservableList from package:observe --- modules/angular2/pubspec.yaml | 1 + .../src/core/compiler/element_injector.ts | 2 + .../src/directives/observable_list_diff.dart | 66 +++++++++++++ modules/angular2/src/test_lib/spies.dart | 5 + .../angular2/src/test_lib/test_injector.ts | 2 + .../core/compiler/integration_dart_spec.dart | 72 +++++++++++++- .../directives/observable_list_diff_spec.dart | 95 +++++++++++++++++++ pubspec.yaml | 2 + 8 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 modules/angular2/src/directives/observable_list_diff.dart create mode 100644 modules/angular2/test/directives/observable_list_diff_spec.dart diff --git a/modules/angular2/pubspec.yaml b/modules/angular2/pubspec.yaml index 0632079a69..5fe4e874c4 100644 --- a/modules/angular2/pubspec.yaml +++ b/modules/angular2/pubspec.yaml @@ -19,6 +19,7 @@ dependencies: source_span: '^1.0.0' stack_trace: '^1.1.1' quiver: '^0.21.4' + observe: '0.13.1' dev_dependencies: guinness: '^0.1.17' transformers: diff --git a/modules/angular2/src/core/compiler/element_injector.ts b/modules/angular2/src/core/compiler/element_injector.ts index a2932e8234..7036ec9a37 100644 --- a/modules/angular2/src/core/compiler/element_injector.ts +++ b/modules/angular2/src/core/compiler/element_injector.ts @@ -603,6 +603,8 @@ export class ElementInjector extends TreeNode implements Depend if (isPresent(dirDep.queryDecorator)) return this._findQuery(dirDep.queryDecorator).list; if (dirDep.key.id === StaticKeys.instance().changeDetectorRefId) { + // We provide the component's view change detector to components and + // the surrounding component's change detector to directives. if (dirBin.metadata.type === DirectiveMetadata.COMPONENT_TYPE) { var componentView = this._preBuiltObjects.view.componentChildViews[this._proto.index]; return componentView.changeDetector.ref; diff --git a/modules/angular2/src/directives/observable_list_diff.dart b/modules/angular2/src/directives/observable_list_diff.dart new file mode 100644 index 0000000000..25caf41a4b --- /dev/null +++ b/modules/angular2/src/directives/observable_list_diff.dart @@ -0,0 +1,66 @@ +library angular2.directives.observable_list_iterable_diff; + +import 'package:observe/observe.dart' show ObservableList; +import 'package:angular2/change_detection.dart'; +import 'package:angular2/src/change_detection/pipes/iterable_changes.dart'; +import 'dart:async'; + +class ObservableListDiff extends IterableChanges { + ChangeDetectorRef _ref; + ObservableListDiff(this._ref); + + bool _updated = true; + ObservableList _collection; + StreamSubscription _subscription; + + bool supports(obj) { + if (obj is ObservableList) return true; + throw "Cannot change the type of a collection"; + } + + onDestroy() { + if (this._subscription != null) { + this._subscription.cancel(); + this._subscription = null; + this._collection = null; + } + } + + dynamic transform(ObservableList collection, [List args]) { + // A new collection instance is passed in. + // - We need to set up a listener. + // - We need to transform collection. + if (!identical(_collection, collection)) { + _collection = collection; + + if (_subscription != null) _subscription.cancel(); + _subscription = collection.changes.listen((_) { + _updated = true; + _ref.requestCheck(); + }); + _updated = false; + return super.transform(collection, args); + + // An update has been registered since the last change detection check. + // - We reset the flag. + // - We diff the collection. + } else if (_updated){ + _updated = false; + return super.transform(collection, args); + + // No updates has been registered. + // Returning this tells change detection that object has not change, + // so it should NOT update the binding. + } else { + return this; + } + } +} + +class ObservableListDiffFactory implements PipeFactory { + const ObservableListDiffFactory(); + bool supports(obj) => obj is ObservableList; + Pipe create(ChangeDetectorRef cdRef) { + return new ObservableListDiff(cdRef); + } +} diff --git a/modules/angular2/src/test_lib/spies.dart b/modules/angular2/src/test_lib/spies.dart index 752aa4492a..e31f2ed501 100644 --- a/modules/angular2/src/test_lib/spies.dart +++ b/modules/angular2/src/test_lib/spies.dart @@ -27,4 +27,9 @@ class SpyPipeFactory extends SpyObject implements PipeFactory { @proxy class SpyDependencyProvider extends SpyObject implements DependencyProvider { noSuchMethod(m) => super.noSuchMethod(m); +} + +@proxy +class SpyChangeDetectorRef extends SpyObject implements ChangeDetectorRef { + noSuchMethod(m) => super.noSuchMethod(m); } \ No newline at end of file diff --git a/modules/angular2/src/test_lib/test_injector.ts b/modules/angular2/src/test_lib/test_injector.ts index 3160824311..4dfafb1f83 100644 --- a/modules/angular2/src/test_lib/test_injector.ts +++ b/modules/angular2/src/test_lib/test_injector.ts @@ -56,6 +56,7 @@ import { DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES } from 'angular2/src/render/dom/dom_renderer'; import {DefaultDomCompiler} from 'angular2/src/render/dom/compiler/compiler'; +import {Log} from './utils'; /** * Returns the root injector bindings. @@ -107,6 +108,7 @@ function _getAppBindings() { CompilerCache, bind(ViewResolver).toClass(MockViewResolver), bind(Pipes).toValue(defaultPipes), + Log, bind(ChangeDetection).toClass(DynamicChangeDetection), ViewLoader, DynamicComponentLoader, diff --git a/modules/angular2/test/core/compiler/integration_dart_spec.dart b/modules/angular2/test/core/compiler/integration_dart_spec.dart index fdc214fd7d..04b52ecdef 100644 --- a/modules/angular2/test/core/compiler/integration_dart_spec.dart +++ b/modules/angular2/test/core/compiler/integration_dart_spec.dart @@ -4,6 +4,9 @@ library angular2.test.di.integration_dart_spec; import 'package:angular2/angular2.dart'; import 'package:angular2/di.dart'; import 'package:angular2/test_lib.dart'; +import 'package:observe/observe.dart'; +import 'package:angular2/src/directives/observable_list_diff.dart'; +import 'package:angular2/src/change_detection/pipes/iterable_changes.dart'; class MockException implements Error { var message; @@ -136,10 +139,52 @@ main() { }); })); }); + + describe("ObservableListDiff", () { + it('should be notified of changes', inject([TestComponentBuilder, Log], fakeAsync((TestComponentBuilder tcb, Log log) { + tcb.overrideView(Dummy, new View( + template: '''''', + directives: [ComponentWithObservableList])) + + .createAsync(Dummy).then((tc) { + tc.componentInstance.value = new ObservableList.from([1,2]); + + tc.detectChanges(); + + expect(log.result()).toEqual("check"); + expect(asNativeElements(tc.componentViewChildren)).toHaveText('12'); + + tc.detectChanges(); + + // we did not change the list => no checks + expect(log.result()).toEqual("check"); + + tc.componentInstance.value.add(3); + + flushMicrotasks(); + + tc.detectChanges(); + + // we changed the list => a check + expect(log.result()).toEqual("check; check"); + expect(asNativeElements(tc.componentViewChildren)).toHaveText('123'); + + // we replaced the list => a check + tc.componentInstance.value = new ObservableList.from([5,6,7]); + + tc.detectChanges(); + + expect(log.result()).toEqual("check; check; check"); + expect(asNativeElements(tc.componentViewChildren)).toHaveText('567'); + }); + }))); + }); } @Component(selector: 'dummy') -class Dummy {} +class Dummy { + dynamic value; +} @Component( selector: 'type-literal-component', @@ -206,3 +251,28 @@ class OnChangeComponent implements OnChange { this.changes = changes; } } + +@Component( + selector: 'component-with-observable-list', + changeDetection: ON_PUSH, + properties: const ['list'], + hostInjector: const [ + const Binding(Pipes, toValue: const Pipes (const {"iterableDiff": const [const ObservableListDiffFactory(), const IterableChangesFactory(), const NullPipeFactory()]})) + ] +) +@View(template: '{{item}}', directives: const [NgFor, DirectiveLoggingChecks]) +class ComponentWithObservableList { + Iterable list; +} + +@Directive( + selector: 'directive-logging-checks', + lifecycle: const [LifecycleEvent.onCheck] +) +class DirectiveLoggingChecks implements OnCheck { + Log log; + + DirectiveLoggingChecks(this.log); + + onCheck() => log.add("check"); +} \ No newline at end of file diff --git a/modules/angular2/test/directives/observable_list_diff_spec.dart b/modules/angular2/test/directives/observable_list_diff_spec.dart new file mode 100644 index 0000000000..6bd63246b5 --- /dev/null +++ b/modules/angular2/test/directives/observable_list_diff_spec.dart @@ -0,0 +1,95 @@ +library angular2.test.directives.observable_list_iterable_diff_spec; + +import 'package:angular2/test_lib.dart'; +import 'package:observe/observe.dart' show ObservableList; +import 'package:angular2/src/directives/observable_list_diff.dart'; + +main() { + describe('ObservableListDiff', () { + var pipeFactory, changeDetectorRef; + + beforeEach(() { + pipeFactory = const ObservableListDiffFactory(); + changeDetectorRef = new SpyChangeDetectorRef(); + }); + + describe("supports", () { + it("should be true for ObservableList", () { + expect(pipeFactory.supports(new ObservableList())).toBe(true); + }); + + it("should be false otherwise", () { + expect(pipeFactory.supports([1,2,3])).toBe(false); + }); + }); + + it("should return the wrapped value to trigger change detection on first invocation of transform", () { + final pipe = pipeFactory.create(changeDetectorRef); + final c = new ObservableList.from([1,2]); + expect(pipe.transform(c, []).wrapped).toBe(pipe); + }); + + it("should return itself when no changes between the calls", () { + final pipe = pipeFactory.create(changeDetectorRef); + + final c = new ObservableList.from([1,2]); + + pipe.transform(c, []); + + expect(pipe.transform(c, [])).toBe(pipe); + }); + + it("should return the wrapped value once a change has been trigger", fakeAsync(() { + final pipe = pipeFactory.create(changeDetectorRef); + + final c = new ObservableList.from([1,2]); + + pipe.transform(c, []); + + c.add(3); + + // same value, because we have not detected the change yet + expect(pipe.transform(c, [])).toBe(pipe); + + // now we detect the change + flushMicrotasks(); + expect(pipe.transform(c, []).wrapped).toBe(pipe); + })); + + it("should request a change detection check upon receiving a change", fakeAsync(() { + final pipe = pipeFactory.create(changeDetectorRef); + + final c = new ObservableList.from([1,2]); + pipe.transform(c, []); + + c.add(3); + flushMicrotasks(); + + expect(changeDetectorRef.spy("requestCheck")).toHaveBeenCalledOnce(); + })); + + it("should return the wrapped value after changing a collection", () { + final pipe = pipeFactory.create(changeDetectorRef); + + final c1 = new ObservableList.from([1,2]); + final c2 = new ObservableList.from([3,4]); + + expect(pipe.transform(c1, []).wrapped).toBe(pipe); + expect(pipe.transform(c2, []).wrapped).toBe(pipe); + }); + + it("should not unbsubscribe from the stream of chagnes after changing a collection", () { + final pipe = pipeFactory.create(changeDetectorRef); + + final c1 = new ObservableList.from([1,2]); + expect(pipe.transform(c1, []).wrapped).toBe(pipe); + + final c2 = new ObservableList.from([3,4]); + expect(pipe.transform(c2, []).wrapped).toBe(pipe); + + // pushing into the first collection has no effect, and we do not see the change + c1.add(3); + expect(pipe.transform(c2, [])).toBe(pipe); + }); + }); +} \ No newline at end of file diff --git a/pubspec.yaml b/pubspec.yaml index 110de16b6a..ace60f941e 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,8 @@ name: angular environment: sdk: '>=1.9.0 <2.0.0' +dependencies: + observe: '0.13.1' dev_dependencies: guinness: '^0.1.17' intl: '^0.12.4'