From 4cadae01bffcb8c76c52f1f101e327547e748caa Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Sat, 21 Jun 2014 11:21:04 +0200 Subject: [PATCH] feat(dccd): add Support for ObservableList, ObservableMap & ChangeNotifier Lower the target time to prevent a memory overflow. Benchmark added. Fixes #773 Closes #1156 --- benchmark/_perf.dart | 17 +- benchmark/watch_group_perf.dart | 271 ++++++- example/pubspec.yaml | 1 + .../dirty_checking_change_detector.dart | 152 +++- lib/tools/transformer/metadata_generator.dart | 14 +- .../transformer/static_angular_generator.dart | 7 +- lib/transformer.dart | 2 + pubspec.lock | 46 +- pubspec.yaml | 3 +- .../dirty_checking_change_detector_spec.dart | 760 +++++++++++++----- 10 files changed, 942 insertions(+), 331 deletions(-) diff --git a/benchmark/_perf.dart b/benchmark/_perf.dart index aaa7437f4..b534a7eed 100644 --- a/benchmark/_perf.dart +++ b/benchmark/_perf.dart @@ -8,7 +8,7 @@ time(name, body) { print('$name: => ${statMeasure(body)}'); } -statMeasure(body) { +StatSample statMeasure(body) { var list = []; var count = 100; for(var i = 0; i < count; i++) { @@ -18,11 +18,11 @@ statMeasure(body) { return new StatSample(list); } -measure(b) { +Sample measure(b) { // actual test; var count = 0; var stopwatch = new Stopwatch(); - var targetTime = 50 * 1000; + var targetTime = 500; stopwatch.start(); do { b(); @@ -49,6 +49,9 @@ measure(b) { b(); b(); b(); b(); b(); b(); b(); b(); b(); b(); // 8 b(); b(); b(); b(); b(); b(); b(); b(); b(); b(); // 9 } + for(var i = 0; i < count % 100; i++) { + b(); + } } stopwatch.stop(); int elapsed = max(1, stopwatch.elapsedMicroseconds); @@ -98,12 +101,12 @@ class StatSample { } class Sample { - num count; - num time_us; + final num count; + final num time_us; Sample(this.count, this.time_us); - get rate => count / time_us; + num get rate => count / time_us; - toString() => rate; + String toString() => '$rate'; } diff --git a/benchmark/watch_group_perf.dart b/benchmark/watch_group_perf.dart index 99bf17f69..5db33e498 100644 --- a/benchmark/watch_group_perf.dart +++ b/benchmark/watch_group_perf.dart @@ -6,6 +6,11 @@ import 'package:angular/change_detection/dirty_checking_change_detector_dynamic. import 'package:angular/change_detection/dirty_checking_change_detector_static.dart'; import 'package:angular/change_detection/watch_group.dart'; import 'package:benchmark_harness/benchmark_harness.dart'; +import 'package:observe/observe.dart'; +import 'package:observe/mirrors_used.dart'; +import "dart:io"; +import "dart:async"; +import "dart:convert"; @MirrorsUsed( targets: const [ @@ -25,62 +30,135 @@ var _staticFieldGetterFactory = new StaticFieldGetterFactory({ var _dynamicFieldGetterFactory = new DynamicFieldGetterFactory(); main() { - _fieldRead(); - _fieldReadGetter(); + _collectionRead(observable:false, changeCount:0); + _collectionRead(observable:true, changeCount:0); + + _collectionRead(observable:false, changeCount:1); + _collectionRead(observable:true, changeCount:1); + + _collectionRead(observable:false, changeCount:10); + _collectionRead(observable:true, changeCount:10); + + _groupAddRemove(observable:false); + _groupAddRemove(observable:true); + + _fieldRead(observable:false, staticGetter: false, changeCount: 0); + _fieldRead(observable:true, staticGetter: false, changeCount: 0); + + _fieldRead(observable:false, staticGetter: false, changeCount: 1); + _fieldRead(observable:true, staticGetter: false, changeCount: 1); + + _fieldRead(observable:false, staticGetter: false, changeCount: 10); + _fieldRead(observable:true, staticGetter: false, changeCount: 10); + + _fieldRead(observable:false, staticGetter: true, changeCount: 0); + _fieldRead(observable:true, staticGetter: true, changeCount: 0); + + _fieldRead(observable:false, staticGetter: true, changeCount: 1); + _fieldRead(observable:true, staticGetter: true, changeCount: 1); + + _fieldRead(observable:false, staticGetter: true, changeCount: 10); + _fieldRead(observable:true, staticGetter: true, changeCount: 10); + _mapRead(); _methodInvoke0(); _methodInvoke1(); _function2(); - new _CollectionCheck().report(); } -class _CollectionCheck extends BenchmarkBase { - List list = new List.generate(1000, (i) => i); +void _collectionRead({bool observable, int changeCount}) { var detector = new DirtyCheckingChangeDetector(_dynamicFieldGetterFactory); - _CollectionCheck(): super('change-detect List[1000]') { - detector - ..watch(list, null, 'handler') - ..collectChanges(); // intialize - } + var description = ''; - run() { - detector.collectChanges(); + List list = new List.generate(20, (i) => i); + + if (observable) { + list = new ObservableList.from(list); + description += '[CHANGE NOTIFIER]'; + } else { + description += '[DIRTY CHECK]'; + } + if (changeCount == 0) { + description += ' 0/${list.length} change'; + } else if (changeCount == 1) { + description += ' 1/${list.length} change'; + } else if (changeCount > 1) { + description += ' $changeCount/${list.length} changes'; } -} -_fieldRead() { - var watchGrp = new RootWatchGroup(_dynamicFieldGetterFactory, - new DirtyCheckingChangeDetector(_dynamicFieldGetterFactory), new _Obj()) - ..watch(_parse('a'), _reactionFn) - ..watch(_parse('b'), _reactionFn) - ..watch(_parse('c'), _reactionFn) - ..watch(_parse('d'), _reactionFn) - ..watch(_parse('e'), _reactionFn) - ..watch(_parse('f'), _reactionFn) - ..watch(_parse('g'), _reactionFn) - ..watch(_parse('h'), _reactionFn) - ..watch(_parse('i'), _reactionFn) - ..watch(_parse('j'), _reactionFn) - ..watch(_parse('k'), _reactionFn) - ..watch(_parse('l'), _reactionFn) - ..watch(_parse('m'), _reactionFn) - ..watch(_parse('n'), _reactionFn) - ..watch(_parse('o'), _reactionFn) - ..watch(_parse('p'), _reactionFn) - ..watch(_parse('q'), _reactionFn) - ..watch(_parse('r'), _reactionFn) - ..watch(_parse('s'), _reactionFn) - ..watch(_parse('t'), _reactionFn); + var watch = detector.watch(list, null, 'handler'); + detector.collectChanges(); - print('Watch: ${watchGrp.fieldCost}; eval: ${watchGrp.evalCost}'); + time(description + (observable ? ' ObservableList' : ' List'), () { + detector.collectChanges(); + if (changeCount == 1) { + list[3]++; + } else if (changeCount > 1) { + for (var i = 0; i < list.length; i++) { + list[i]++; + } + } + }); +} - time('fieldRead', () => watchGrp.detectChanges()); +void _groupAddRemove({bool observable}) { + var description = ''; + if (observable) { + description += '[CHANGE NOTIFIER]'; + } else { + description += '[DIRTY CHECK]'; + } + var rootWatchGrp = new RootWatchGroup(_staticFieldGetterFactory, + new DirtyCheckingChangeDetector(_staticFieldGetterFactory), {}); + var testRun = () { + for (int i = 0; i < 10; i++) { + WatchGroup child = rootWatchGrp.newGroup(observable ? new _ObservableObj() : new _Obj()) + ..watch(_parse('a'), _reactionFn) + ..watch(_parse('b'), _reactionFn) + ..watch(_parse('c'), _reactionFn) + ..watch(_parse('d'), _reactionFn) + ..watch(_parse('e'), _reactionFn) + ..watch(_parse('f'), _reactionFn) + ..watch(_parse('g'), _reactionFn) + ..watch(_parse('h'), _reactionFn) + ..watch(_parse('i'), _reactionFn) + ..watch(_parse('j'), _reactionFn); + rootWatchGrp.detectChanges(); + child.remove(); + } + + }; + time(description + ' add/remove 10 watchGroups with 10 watches', testRun); } -_fieldReadGetter() { - var watchGrp= new RootWatchGroup(_staticFieldGetterFactory, - new DirtyCheckingChangeDetector(_staticFieldGetterFactory), new _Obj()) +void _fieldRead({bool observable, bool staticGetter, int changeCount}) { + var description = ''; + var obj; + if (observable) { + description += '[CHANGE NOTIFIER]'; + obj = new _ObservableObj(); + } else { + description += '[DIRTY CHECK]'; + obj = new _Obj(); + } + var fieldGetterFactory; + if (staticGetter) { + description += ' staticGetter'; + fieldGetterFactory = _staticFieldGetterFactory; + } else { + description += ' dynamicGetter'; + fieldGetterFactory = _dynamicFieldGetterFactory; + } + if (changeCount == 0) { + description +=' 0/20 field change'; + } else if (changeCount == 1) { + description +=' 1/20 field change'; + } else if (changeCount > 1) { + description +=' 20/20 field changes'; + } + var watchGrp = new RootWatchGroup(fieldGetterFactory, + new DirtyCheckingChangeDetector(fieldGetterFactory), obj) ..watch(_parse('a'), _reactionFn) ..watch(_parse('b'), _reactionFn) ..watch(_parse('c'), _reactionFn) @@ -104,7 +182,33 @@ _fieldReadGetter() { print('Watch: ${watchGrp.fieldCost}; eval: ${watchGrp.evalCost}'); - time('fieldReadGetter', () => watchGrp.detectChanges()); + time(description + ' on the same object', () { + if (changeCount == 1) { + obj.c++; + } else if (changeCount > 1) { + obj.a++; + obj.b++; + obj.c++; + obj.d++; + obj.e++; + obj.f++; + obj.g++; + obj.h++; + obj.i++; + obj.j++; + obj.k++; + obj.l++; + obj.m++; + obj.n++; + obj.o++; + obj.p++; + obj.q++; + obj.r++; + obj.s++; + obj.t++; + } + watchGrp.detectChanges(); + }); } _mapRead() { @@ -313,5 +417,86 @@ class _Obj { methodR() => r; methodS() => s; methodT() => t; - } + +class _ObservableObj extends ChangeNotifier { + @reflectable @observable dynamic get a => __$a; + dynamic __$a = 1; + @reflectable set a(dynamic value) { __$a = notifyPropertyChange(#a, __$a, value); } + + @reflectable @observable dynamic get b => __$b; + dynamic __$b = 2; + @reflectable set b(dynamic value) { __$b = notifyPropertyChange(#b, __$b, value); } + + @reflectable @observable dynamic get c => __$c; + dynamic __$c = 3; + @reflectable set c(dynamic value) { __$c = notifyPropertyChange(#c, __$c, value); } + + @reflectable @observable dynamic get d => __$d; + dynamic __$d = 4; + @reflectable set d(dynamic value) { __$d = notifyPropertyChange(#d, __$d, value); } + + @reflectable @observable dynamic get e => __$e; + dynamic __$e = 5; + @reflectable set e(dynamic value) { __$e = notifyPropertyChange(#e, __$e, value); } + + @reflectable @observable dynamic get f => __$f; + dynamic __$f = 6; + @reflectable set f(dynamic value) { __$f = notifyPropertyChange(#f, __$f, value); } + + @reflectable @observable dynamic get g => __$g; + dynamic __$g = 7; + @reflectable set g(dynamic value) { __$g = notifyPropertyChange(#g, __$g, value); } + + @reflectable @observable dynamic get h => __$h; + dynamic __$h = 8; + @reflectable set h(dynamic value) { __$h = notifyPropertyChange(#h, __$h, value); } + + @reflectable @observable dynamic get i => __$i; + dynamic __$i = 9; + @reflectable set i(dynamic value) { __$i = notifyPropertyChange(#i, __$i, value); } + + @reflectable @observable dynamic get j => __$j; + dynamic __$j = 10; + @reflectable set j(dynamic value) { __$j = notifyPropertyChange(#j, __$j, value); } + + @reflectable @observable dynamic get k => __$k; + dynamic __$k = 11; + @reflectable set k(dynamic value) { __$k = notifyPropertyChange(#k, __$k, value); } + + @reflectable @observable dynamic get l => __$l; + dynamic __$l = 12; + @reflectable set l(dynamic value) { __$l = notifyPropertyChange(#l, __$l, value); } + + @reflectable @observable dynamic get m => __$m; + dynamic __$m = 13; + @reflectable set m(dynamic value) { __$m = notifyPropertyChange(#m, __$m, value); } + + @reflectable @observable dynamic get n => __$n; + dynamic __$n = 14; + @reflectable set n(dynamic value) { __$n = notifyPropertyChange(#n, __$n, value); } + + @reflectable @observable dynamic get o => __$o; + dynamic __$o = 15; + @reflectable set o(dynamic value) { __$o = notifyPropertyChange(#o, __$o, value); } + + @reflectable @observable dynamic get p => __$p; + dynamic __$p = 16; + @reflectable set p(dynamic value) { __$p = notifyPropertyChange(#p, __$p, value); } + + @reflectable @observable dynamic get q => __$q; + dynamic __$q = 17; + @reflectable set q(dynamic value) { __$q = notifyPropertyChange(#q, __$q, value); } + + @reflectable @observable dynamic get r => __$r; + dynamic __$r = 18; + @reflectable set r(dynamic value) { __$r = notifyPropertyChange(#r, __$r, value); } + + @reflectable @observable dynamic get s => __$s; + dynamic __$s = 19; + @reflectable set s(dynamic value) { __$s = notifyPropertyChange(#s, __$s, value); } + + @reflectable @observable dynamic get t => __$t; + dynamic __$t = 20; + @reflectable set t(dynamic value) { __$t = notifyPropertyChange(#t, __$t, value); } +} \ No newline at end of file diff --git a/example/pubspec.yaml b/example/pubspec.yaml index 56d641013..a0fea3578 100644 --- a/example/pubspec.yaml +++ b/example/pubspec.yaml @@ -9,3 +9,4 @@ dependencies: transformers: - angular + diff --git a/lib/change_detection/dirty_checking_change_detector.dart b/lib/change_detection/dirty_checking_change_detector.dart index 0fec28340..45f62c517 100644 --- a/lib/change_detection/dirty_checking_change_detector.dart +++ b/lib/change_detection/dirty_checking_change_detector.dart @@ -2,6 +2,8 @@ library dirty_checking_change_detector; import 'dart:collection'; import 'package:angular/change_detection/change_detection.dart'; +import 'dart:async'; +import 'package:observe/observe.dart' as obs; /** * [DirtyCheckingChangeDetector] determines which object properties have changed @@ -46,6 +48,9 @@ class DirtyCheckingChangeDetectorGroup implements ChangeDetectorGroup { */ DirtyCheckingRecord _recordHead, _recordTail; + /// Registry of record with their notifiers + Map, StreamSubscription> _observableRecords; + /** * Same as [_tail] but includes child-group records as well. */ @@ -116,8 +121,7 @@ class DirtyCheckingChangeDetectorGroup implements ChangeDetectorGroup { WatchRecord watch(Object object, String field, H handler) { assert(_root != null); // prove that we are not deleted connected; - return _recordAdd(new DirtyCheckingRecord(this, _fieldGetterFactory, - handler, field, object)); + return _recordAdd(new DirtyCheckingRecord(this, _fieldGetterFactory, handler, field, object)); } /** @@ -146,6 +150,7 @@ class DirtyCheckingChangeDetectorGroup implements ChangeDetectorGroup { var root; assert((root = _root) != null); assert(root._assertRecordsOk()); + _cancelChildObservables(); DirtyCheckingRecord prevRecord = _recordHead._prevRecord; var childInclRecordTail = _childInclRecordTail; DirtyCheckingRecord nextRecord = childInclRecordTail._nextRecord; @@ -191,6 +196,8 @@ class DirtyCheckingChangeDetectorGroup implements ChangeDetectorGroup { } void _recordRemove(DirtyCheckingRecord record) { + _cancelObservable(record); + DirtyCheckingRecord previous = record._prevRecord; DirtyCheckingRecord next = record._nextRecord; @@ -209,6 +216,35 @@ class DirtyCheckingChangeDetectorGroup implements ChangeDetectorGroup { } } + /// Register an observable object in this group. + void _registerObservable(DirtyCheckingRecord record, StreamSubscription subscription) { + if (_observableRecords == null) _observableRecords = new HashMap.identity(); + _observableRecords[record] = subscription; + } + + /// Cancel an observable subscription in this group. + void _cancelObservable(DirtyCheckingRecord record) { + if (_observableRecords == null) return; + var subscription = _observableRecords.remove(record); + if (subscription != null) subscription.cancel(); + } + + /// Cancel all observable subscriptions in this group. + void _cancelOwnObservables() { + if (_observableRecords != null) { + _observableRecords.values.forEach((s) => s.cancel()); + _observableRecords = null; + } + } + + /// Cancel all observable subscriptions in this group and descendant groups + void _cancelChildObservables() { + this._cancelOwnObservables(); + for (var child = _childHead; child != null; child = child._next) { + child._cancelChildObservables(); + } + } + String toString() { var lines = []; if (_parent == null) { @@ -306,6 +342,7 @@ class DirtyCheckingChangeDetector extends DirtyCheckingChangeDetectorGroup Iterator> collectChanges({EvalExceptionHandler exceptionHandler, AvgStopwatch stopwatch}) { if (stopwatch != null) stopwatch.start(); + DirtyCheckingRecord changeTail = _fakeHead; DirtyCheckingRecord current = _recordHead; // current index @@ -375,18 +412,26 @@ class DirtyCheckingRecord implements Record, WatchRecord { 'NOOP', 'IDENTITY', 'GETTER', - 'GETTER / CLOSURE' + 'NOTIFIED GETTER', + 'GETTER / CLOSURE', + 'OBSERVABLE GETTER / CLOSURE', 'MAP[]', 'ITERABLE', - 'MAP']; + 'NOTIFIED LIST', + 'MAP', + 'NOTIFIED MAP']; static const int _MODE_MARKER_ = 0; static const int _MODE_NOOP_ = 1; static const int _MODE_IDENTITY_ = 2; static const int _MODE_GETTER_ = 3; - static const int _MODE_GETTER_OR_METHOD_CLOSURE_ = 4; - static const int _MODE_MAP_FIELD_ = 5; - static const int _MODE_ITERABLE_ = 6; - static const int _MODE_MAP_ = 7; + static const int _MODE_GETTER_NOTIFIED_ = 4; + static const int _MODE_GETTER_OR_METHOD_CLOSURE_ = 5; + static const int _MODE_GETTER_OBS_OR_METHOD_CLOSURE_ = 6; + static const int _MODE_MAP_FIELD_ = 7; + static const int _MODE_ITERABLE_ = 8; + static const int _MODE_LIST_NOTIFIED_ = 9; + static const int _MODE_MAP_ = 10; + static const int _MODE_MAP_NOTIFIED_ = 11; final DirtyCheckingChangeDetectorGroup _group; final FieldGetterFactory _fieldGetterFactory; @@ -403,8 +448,7 @@ class DirtyCheckingRecord implements Record, WatchRecord { var _object; FieldGetter _getter; - DirtyCheckingRecord(this._group, this._fieldGetterFactory, this.handler, - this.field, _object) { + DirtyCheckingRecord(this._group, this._fieldGetterFactory, this.handler, this.field, _object) { object = _object; } @@ -424,7 +468,10 @@ class DirtyCheckingRecord implements Record, WatchRecord { * reflection. If [Map] then it sets up map accessor. */ void set object(obj) { + _group._cancelObservable(this); + _object = obj; + if (obj == null) { _mode = _MODE_IDENTITY_; _getter = null; @@ -434,30 +481,41 @@ class DirtyCheckingRecord implements Record, WatchRecord { if (field == null) { _getter = null; if (obj is Map) { - if (_mode != _MODE_MAP_) { - _mode = _MODE_MAP_; + if (currentValue is! _MapChangeRecord) { currentValue = new _MapChangeRecord(); - } - if (currentValue.isDirty) { - // We're dirty because the mapping we tracked by reference mutated. - // In addition, our reference has now changed. We should compare the - // previous reported value of that mapping with the one from the - // new reference. + } else if (currentValue.isDirty) { + // We're dirty because the mapping we tracked by reference mutated. In addition, our + // reference has now changed. We should compare the previous reported value of that + // mapping with the one from the new reference. currentValue._revertToPreviousState(); } - + if (obj is obs.ChangeNotifier) { + _mode = _MODE_MAP_NOTIFIED_; // Run the dccd after the map is added + var subscription = (obj as obs.ChangeNotifier).changes.listen((_) { + _mode = _MODE_MAP_NOTIFIED_; // Run the dccd after the map is updated + }); + _group._registerObservable(this, subscription); + } else { + _mode = _MODE_MAP_; + } } else if (obj is Iterable) { - if (_mode != _MODE_ITERABLE_) { - _mode = _MODE_ITERABLE_; + if (currentValue is! _CollectionChangeRecord) { currentValue = new _CollectionChangeRecord(); - } - if (currentValue.isDirty) { - // We're dirty because the collection we tracked by reference mutated. - // In addition, our reference has now changed. We should compare the - // previous reported value of that collection with the one from the - // new reference. + } else if (currentValue.isDirty) { + // We're dirty because the collection we tracked by reference mutated. In addition, our + // reference has now changed. We should compare the previous reported value of that + // collection with the one from the new reference. currentValue._revertToPreviousState(); } + if (obj is obs.ObservableList) { + _mode = _MODE_LIST_NOTIFIED_; // Run the dccd after the list is added + var subscription = (obj as obs.ObservableList).listChanges.listen((_) { + _mode = _MODE_LIST_NOTIFIED_; // Run the dccd after the list is updated + }); + _group._registerObservable(this, subscription); + } else { + _mode = _MODE_ITERABLE_; + } } else { _mode = _MODE_IDENTITY_; } @@ -469,7 +527,9 @@ class DirtyCheckingRecord implements Record, WatchRecord { _mode = _MODE_MAP_FIELD_; _getter = null; } else { - _mode = _MODE_GETTER_OR_METHOD_CLOSURE_; + _mode = obj is obs.ChangeNotifier ? + _MODE_GETTER_OBS_OR_METHOD_CLOSURE_ : + _MODE_GETTER_OR_METHOD_CLOSURE_; _getter = _fieldGetterFactory.getter(obj, field); } } @@ -485,12 +545,15 @@ class DirtyCheckingRecord implements Record, WatchRecord { case _MODE_GETTER_: current = _getter(object); break; + case _MODE_GETTER_NOTIFIED_: + _mode = _MODE_NOOP_; // no-op until next notification + current = _getter(object); + break; case _MODE_GETTER_OR_METHOD_CLOSURE_: - // NOTE: When Dart looks up a method "foo" on object "x", it returns a - // new closure for each lookup. They compare equal via "==" but are no - // identical(). There's no point getting a new value each time and - // decide it's the same so we'll skip further checking after the first - // time. + // NOTE: When Dart looks up a method "foo" on object "x", it returns a new closure for each + // lookup. They compare equal via "==" but are no identical(). There's no point getting a + // new value each time and decide it's the same so we'll skip further checking after the + // first time. current = _getter(object); if (current is Function && !identical(current, _getter(object))) { _mode = _MODE_NOOP_; @@ -498,6 +561,20 @@ class DirtyCheckingRecord implements Record, WatchRecord { _mode = _MODE_GETTER_; } break; + case _MODE_GETTER_OBS_OR_METHOD_CLOSURE_: + current = _getter(object); + // no-op if closure (see _MODE_GETTER_OR_METHOD_CLOSURE_) or until the next notification + _mode = _MODE_NOOP_; + if (current is! Function || identical(current, _getter(object))) { + var subscription = (object as obs.Observable).changes.listen((records) { + // todo(vicb) we should only go to the _MODE_GETTER_NOTIFIED_ mode when a record + // is applicable to the current `field`. With the current implementation, any field + // on an observable object will trigger this listener. + _mode = _MODE_GETTER_NOTIFIED_; + }); + _group._registerObservable(this, subscription); + } + break; case _MODE_MAP_FIELD_: current = object[field]; break; @@ -505,8 +582,14 @@ class DirtyCheckingRecord implements Record, WatchRecord { current = object; _mode = _MODE_NOOP_; break; + case _MODE_MAP_NOTIFIED_: + _mode = _MODE_NOOP_; // no-op until next notification + return (currentValue as _MapChangeRecord)._check(object); case _MODE_MAP_: return (currentValue as _MapChangeRecord)._check(object); + case _MODE_LIST_NOTIFIED_: + _mode = _MODE_NOOP_; // no-op until next notification + return (currentValue as _CollectionChangeRecord)._check(object); case _MODE_ITERABLE_: return (currentValue as _CollectionChangeRecord)._check(object); default: @@ -522,7 +605,6 @@ class DirtyCheckingRecord implements Record, WatchRecord { return false; } - void remove() { _group._recordRemove(this); } @@ -531,8 +613,6 @@ class DirtyCheckingRecord implements Record, WatchRecord { '${_mode < _MODE_NAMES.length ? _MODE_NAMES[_mode] : '?'}[$field]{$hashCode}'; } -final Object _INITIAL_ = new Object(); - class _MapChangeRecord implements MapChangeRecord { final _records = new HashMap(); Map _map; diff --git a/lib/tools/transformer/metadata_generator.dart b/lib/tools/transformer/metadata_generator.dart index bd841967b..eeed3cd03 100644 --- a/lib/tools/transformer/metadata_generator.dart +++ b/lib/tools/transformer/metadata_generator.dart @@ -18,13 +18,11 @@ class MetadataGenerator extends Transformer with ResolverTransformer { void applyResolver(Transform transform, Resolver resolver) { var asset = transform.primaryInput; var id = asset.id; - var outputFilename = '${path.url.basenameWithoutExtension(id.path)}' - '_static_metadata.dart'; + var outputFilename = '${path.url.basenameWithoutExtension(id.path)}_static_metadata.dart'; var outputPath = path.url.join(path.url.dirname(id.path), outputFilename); var outputId = new AssetId(id.package, outputPath); - var extractor = new AnnotationExtractor(transform.logger, resolver, - outputId); + var extractor = new AnnotationExtractor(transform.logger, resolver, outputId); var outputBuffer = new StringBuffer(); _writeHeader(asset.id, outputBuffer); @@ -57,14 +55,12 @@ class MetadataGenerator extends Transformer with ResolverTransformer { _writeClassPreamble(outputBuffer); for (var type in annotatedTypes) { - type.writeClassAnnotations( - outputBuffer, transform.logger, resolver, importPrefixes); + type.writeClassAnnotations(outputBuffer, transform.logger, resolver, importPrefixes); } _writeClassEpilogue(outputBuffer); - transform.addOutput( - new Asset.fromString(outputId, outputBuffer.toString())); - transform.addOutput(asset); + transform..addOutput(new Asset.fromString(outputId, outputBuffer.toString())) + ..addOutput(asset); } } diff --git a/lib/tools/transformer/static_angular_generator.dart b/lib/tools/transformer/static_angular_generator.dart index 69f130eea..75f0c290f 100644 --- a/lib/tools/transformer/static_angular_generator.dart +++ b/lib/tools/transformer/static_angular_generator.dart @@ -18,8 +18,7 @@ class StaticAngularGenerator extends Transformer with ResolverTransformer { void applyResolver(Transform transform, Resolver resolver) { var asset = transform.primaryInput; - var dynamicApp = - resolver.getLibraryFunction('angular.app.factory.applicationFactory'); + var dynamicApp = resolver.getLibraryFunction('angular.app.factory.applicationFactory'); if (dynamicApp == null) { // No dynamic app imports, exit. transform.addOutput(transform.primaryInput); @@ -40,11 +39,11 @@ class StaticAngularGenerator extends Transformer with ResolverTransformer { } } - var dynamicToStatic = - new _NgDynamicToStaticVisitor(dynamicApp, transaction); + var dynamicToStatic = new _NgDynamicToStaticVisitor(dynamicApp, transaction); unit.accept(dynamicToStatic); var generatedFilePrefix = '${path.url.basenameWithoutExtension(id.path)}'; + _addImport(transaction, unit, '${generatedFilePrefix}_static_expressions.dart', 'generated_static_expressions'); diff --git a/lib/transformer.dart b/lib/transformer.dart index f8637a8ea..9fbcbcd54 100644 --- a/lib/transformer.dart +++ b/lib/transformer.dart @@ -11,6 +11,7 @@ import 'package:barback/barback.dart'; import 'package:code_transformers/resolver.dart'; import 'package:di/transformer.dart' as di; import 'package:path/path.dart' as path; +import 'package:observe/transformer.dart' show ObservableTransformer; /** @@ -127,6 +128,7 @@ Transformer _staticGenerator(TransformOptions options) { List> _createPhases(TransformOptions options) => [ + [ new ObservableTransformer() ], [ new HtmlDartReferencesGenerator(options) ], [ new di.InjectorGenerator(options.diOptions, new Resolvers(options.sdkDirectory)) ], [ _staticGenerator(options) ] diff --git a/pubspec.lock b/pubspec.lock index 1b6409e97..dd74235d2 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -4,7 +4,7 @@ packages: analyzer: description: analyzer source: hosted - version: "0.18.0" + version: "0.15.7" args: description: args source: hosted @@ -12,7 +12,7 @@ packages: barback: description: barback source: hosted - version: "0.14.1+3" + version: "0.14.0+3" benchmark_harness: description: benchmark_harness source: hosted @@ -24,7 +24,7 @@ packages: code_transformers: description: code_transformers source: hosted - version: "0.1.5" + version: "0.1.6" collection: description: collection source: hosted @@ -32,11 +32,11 @@ packages: di: description: di source: hosted - version: "2.0.1" + version: "2.0.2" guinness: description: guinness source: hosted - version: "0.1.9" + version: "0.1.14" html5lib: description: html5lib source: hosted @@ -44,7 +44,7 @@ packages: intl: description: intl source: hosted - version: "0.8.10+4" + version: "0.11.6" js: description: js source: hosted @@ -52,23 +52,23 @@ packages: logging: description: logging source: hosted - version: "0.9.1+1" + version: "0.9.2" matcher: description: matcher source: hosted - version: "0.11.0" - meta: - description: meta - source: hosted - version: "0.8.8" + version: "0.11.1" mock: description: mock source: hosted version: "0.11.0+2" + observe: + description: observe + source: hosted + version: "0.10.1+2" path: description: path source: hosted - version: "1.2.1" + version: "1.3.0" perf_api: description: perf_api source: hosted @@ -81,26 +81,30 @@ packages: description: route_hierarchical source: hosted version: "0.4.22" + smoke: + description: smoke + source: hosted + version: "0.1.0+1" source_maps: description: source_maps source: hosted - version: "0.9.3" + version: "0.9.4" + source_span: + description: source_span + source: hosted + version: "1.0.0" stack_trace: description: stack_trace source: hosted - version: "1.0.2" - typed_mock: - description: typed_mock - source: hosted - version: "0.0.4" + version: "0.9.3+2" unittest: description: unittest source: hosted - version: "0.11.0+3" + version: "0.11.0+5" utf: description: utf source: hosted - version: "0.9.0" + version: "0.9.0+1" web_components: description: web_components source: hosted diff --git a/pubspec.yaml b/pubspec.yaml index b9a8f14e5..614efd581 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -13,8 +13,8 @@ homepage: https://angulardart.org environment: sdk: '>=1.4.0' dependencies: - args: '>=0.10.0 <0.11.0' analyzer: '>=0.15.0 <0.19.0' + args: '>=0.10.0 <0.11.0' barback: '>=0.13.0 <0.17.0' browser: '>=0.10.0 <0.11.0' code_transformers: '>=0.1.4+2 <0.2.0' @@ -22,6 +22,7 @@ dependencies: di: '>=2.0.1 <3.0.0' html5lib: '>=0.10.0 <0.11.0' intl: '>=0.8.7 <0.12.0' + observe: '>=0.10.0+1 <0.11.0' perf_api: '>=0.0.9 <0.1.0' route_hierarchical: '>=0.4.22 <0.5.0' dev_dependencies: diff --git a/test/change_detection/dirty_checking_change_detector_spec.dart b/test/change_detection/dirty_checking_change_detector_spec.dart index 3fcabaa7d..6f68091a9 100644 --- a/test/change_detection/dirty_checking_change_detector_spec.dart +++ b/test/change_detection/dirty_checking_change_detector_spec.dart @@ -7,10 +7,16 @@ import 'package:angular/change_detection/dirty_checking_change_detector_static.d import 'package:angular/change_detection/dirty_checking_change_detector_dynamic.dart'; import 'dart:collection'; import 'dart:math'; +import 'package:observe/observe.dart' + show ObservableList, + ObservableMap, + ChangeNotifier, + observable; + +DirtyCheckingChangeDetector detector; void testWithGetterFactory(FieldGetterFactory getterFactory) { describe('DirtyCheckingChangeDetector with ${getterFactory.runtimeType}', () { - DirtyCheckingChangeDetector detector; beforeEach(() { detector = new DirtyCheckingChangeDetector(getterFactory); @@ -22,6 +28,38 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) { expect(changes.moveNext()).toEqual(false); }); + it('should detect observable field changes', async(() { + var user = new _ObservableUser('', ''); + Iterator changeIterator; + + detector..watch(user, 'first', null) + ..watch(user, 'last', null) + ..collectChanges(); // throw away first set + + changeIterator = detector.collectChanges(); + expect(changeIterator.moveNext()).toEqual(false); + user..first = 'misko' + ..last = 'hevery'; + + microLeap(); + changeIterator = detector.collectChanges(); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue).toEqual('misko'); + expect(changeIterator.current.previousValue).toEqual(''); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue).toEqual('hevery'); + expect(changeIterator.current.previousValue).toEqual(''); + expect(changeIterator.moveNext()).toEqual(false); + + user.first = 'victor'; + microLeap(); + changeIterator = detector.collectChanges(); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue).toEqual('victor'); + expect(changeIterator.current.previousValue).toEqual('misko'); + expect(changeIterator.moveNext()).toEqual(false); + })); + it('should detect field changes', () { var user = new _User('', ''); Iterator changeIterator; @@ -137,6 +175,45 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) { expect(detector.collectChanges(), toEqualChanges(['0a', '0A', '1b'])); }); + it('should cancel notification subscription on group removal', async(() { + var obj = {}; + var observable = new _ObservableUser(); + expect(observable.hasObservers).toEqual(false); + detector.watch(obj, 'a', '0a'); + var child1a = detector.newGroup(); + var child1b = detector.newGroup(); + var child2 = child1a.newGroup(); + child1a.watch(observable,'first', '1a'); + child1b.watch(obj, 'a', '1b'); + detector.watch(obj, 'a', '0A'); + child1a.watch(obj,'a', '1A'); + child2.watch(observable,'last', '2A'); + + var iterator; + obj['a'] = 1; + observable.first = 'foo'; + observable.last = 'bar'; + expect(detector.collectChanges(), + toEqualChanges(['0a', '0A', '1a', '1A', '2A', '1b'])); + expect(observable.hasObservers).toEqual(true); + + obj['a'] = 2; + child1a.remove(); // should also remove child2 + expect(detector.collectChanges(), toEqualChanges(['0a', '0A', '1b'])); + expect(observable.hasObservers).toEqual(false); + })); + + it('should cancel notification subscription on record removal', async(() { + var observable = new _ObservableUser(); + expect(observable.hasObservers).toEqual(false); + var watch = detector.watch(observable, 'first', '0a'); + observable.first = 'foo'; + expect(detector.collectChanges(), toEqualChanges(['0a'])); + expect(observable.hasObservers).toEqual(true); + watch.remove(); + expect(observable.hasObservers).toEqual(false); + })); + it('should add watches within its own group', () { var obj = {}; var ra = detector.watch(obj, 'a', 'a'); @@ -510,218 +587,13 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) { expect(detector.collectChanges().moveNext()).toEqual(false); }); - it('should bug', () { - var list = [1, 2, 3, 4]; - var record = detector.watch(list, null, 'handler'); - var iterator; - - iterator = detector.collectChanges()..moveNext(); - expect(iterator.current.currentValue, toEqualCollectionRecord( - collection: ['1[null -> 0]', '2[null -> 1]', '3[null -> 2]', '4[null -> 3]'], - additions: ['1[null -> 0]', '2[null -> 1]', '3[null -> 2]', '4[null -> 3]'], - moves: [], - removals: [])); - detector.collectChanges(); - - list.removeRange(0, 1); - iterator = detector.collectChanges()..moveNext(); - expect(iterator.current.currentValue, toEqualCollectionRecord( - collection: ['2[1 -> 0]', '3[2 -> 1]', '4[3 -> 2]'], - previous: ['1[0 -> null]', '2[1 -> 0]', '3[2 -> 1]', '4[3 -> 2]'], - additions: [], - moves: ['2[1 -> 0]', '3[2 -> 1]', '4[3 -> 2]'], - removals: ['1[0 -> null]'])); - - list.insert(0, 1); - iterator = detector.collectChanges()..moveNext(); - expect(iterator.current.currentValue, toEqualCollectionRecord( - collection: ['1[null -> 0]', '2[0 -> 1]', '3[1 -> 2]', '4[2 -> 3]'], - previous: ['2[0 -> 1]', '3[1 -> 2]', '4[2 -> 3]'], - additions: ['1[null -> 0]'], - moves: ['2[0 -> 1]', '3[1 -> 2]', '4[2 -> 3]'], - removals: [])); - }); - - it('should properly support objects with equality', () { - FooBar.fooIds = 0; - var list = [new FooBar('a', 'a'), new FooBar('a', 'a')]; - var record = detector.watch(list, null, 'handler'); - var iterator; - - iterator = detector.collectChanges()..moveNext(); - expect(iterator.current.currentValue, toEqualCollectionRecord( - collection: ['(0)a-a[null -> 0]', '(1)a-a[null -> 1]'], - additions: ['(0)a-a[null -> 0]', '(1)a-a[null -> 1]'], - moves: [], - removals: [])); - detector.collectChanges(); - - list.removeRange(0, 1); - iterator = detector.collectChanges()..moveNext(); - expect(iterator.current.currentValue, toEqualCollectionRecord( - collection: ['(1)a-a[1 -> 0]'], - previous: ['(0)a-a[0 -> null]', '(1)a-a[1 -> 0]'], - additions: [], - moves: ['(1)a-a[1 -> 0]'], - removals: ['(0)a-a[0 -> null]'])); - - list.insert(0, new FooBar('a', 'a')); - iterator = detector.collectChanges()..moveNext(); - expect(iterator.current.currentValue, toEqualCollectionRecord( - collection: ['(2)a-a[null -> 0]', '(1)a-a[0 -> 1]'], - previous: ['(1)a-a[0 -> 1]'], - additions: ['(2)a-a[null -> 0]'], - moves: ['(1)a-a[0 -> 1]'], - removals: [])); - }); - - it('should not report unnecessary moves', () { - var list = ['a', 'b', 'c']; - var record = detector.watch(list, null, null); - var iterator = detector.collectChanges()..moveNext(); - - list..clear()..addAll(['b', 'a', 'c']); - iterator = detector.collectChanges()..moveNext(); - expect(iterator.current.currentValue, toEqualCollectionRecord( - collection: ['b[1 -> 0]', 'a[0 -> 1]', 'c'], - previous: ['a[0 -> 1]', 'b[1 -> 0]', 'c'], - additions: [], - moves: ['b[1 -> 0]', 'a[0 -> 1]'], - removals: [])); - }); + addListSpec(useObservable: false); + addListSpec(useObservable: true); }); describe('map watching', () { - describe('previous state', () { - it('should store on insertion', () { - var map = {}; - var record = detector.watch(map, null, null); - expect(detector.collectChanges().moveNext()).toEqual(false); - var iterator; - - map['a'] = 1; - iterator = detector.collectChanges(); - expect(iterator.moveNext()).toEqual(true); - expect(iterator.current.currentValue, toEqualMapRecord( - map: ['a[null -> 1]'], - previous: [], - additions: ['a[null -> 1]'], - changes: [], - removals: [])); - - map['b'] = 2; - iterator = detector.collectChanges(); - expect(iterator.moveNext()).toEqual(true); - expect(iterator.current.currentValue, toEqualMapRecord( - map: ['a', 'b[null -> 2]'], - previous: ['a'], - additions: ['b[null -> 2]'], - changes: [], - removals: [])); - }); - - it('should handle changing key/values correctly', () { - var map = {1: 10, 2: 20}; - var record = detector.watch(map, null, null); - detector.collectChanges().moveNext(); - var iterator; - - map[1] = 20; - map[2] = 10; - iterator = detector.collectChanges(); - expect(iterator.moveNext()).toEqual(true); - expect(iterator.current.currentValue, toEqualMapRecord( - map: ['1[10 -> 20]', '2[20 -> 10]'], - previous: ['1[10 -> 20]', '2[20 -> 10]'], - additions: [], - changes: ['1[10 -> 20]', '2[20 -> 10]'], - removals: [])); - }); - }); - - it('should do basic map watching', () { - var map = {}; - var record = detector.watch(map, null, 'handler'); - expect(detector.collectChanges().moveNext()).toEqual(false); - - var changeIterator; - map['a'] = 'A'; - changeIterator = detector.collectChanges(); - expect(changeIterator.moveNext()).toEqual(true); - expect(changeIterator.current.currentValue, toEqualMapRecord( - map: ['a[null -> A]'], - previous: [], - additions: ['a[null -> A]'], - changes: [], - removals: [])); - - map['b'] = 'B'; - changeIterator = detector.collectChanges(); - expect(changeIterator.moveNext()).toEqual(true); - expect(changeIterator.current.currentValue, toEqualMapRecord( - map: ['a', 'b[null -> B]'], - previous: ['a'], - additions: ['b[null -> B]'], - changes: [], - removals: [])); - - map['b'] = 'BB'; - map['d'] = 'D'; - changeIterator = detector.collectChanges(); - expect(changeIterator.moveNext()).toEqual(true); - expect(changeIterator.current.currentValue, toEqualMapRecord( - map: ['a', 'b[B -> BB]', 'd[null -> D]'], - previous: ['a', 'b[B -> BB]'], - additions: ['d[null -> D]'], - changes: ['b[B -> BB]'], - removals: [])); - - map.remove('b'); - expect(map).toEqual({'a': 'A', 'd':'D'}); - changeIterator = detector.collectChanges(); - expect(changeIterator.moveNext()).toEqual(true); - expect(changeIterator.current.currentValue, toEqualMapRecord( - map: ['a', 'd'], - previous: ['a', 'b[BB -> null]', 'd'], - additions: [], - changes: [], - removals: ['b[BB -> null]'])); - - map.clear(); - changeIterator = detector.collectChanges(); - expect(changeIterator.moveNext()).toEqual(true); - expect(changeIterator.current.currentValue, toEqualMapRecord( - map: [], - previous: ['a[A -> null]', 'd[D -> null]'], - additions: [], - changes: [], - removals: ['a[A -> null]', 'd[D -> null]'])); - }); - - it('should test string keys by value rather than by reference', () { - var map = {'foo': 0}; - detector..watch(map, null, null)..collectChanges(); - - map['f' + 'oo'] = 0; - - expect(detector.collectChanges().moveNext()).toEqual(false); - }); - - it('should test string values by value rather than by reference', () { - var map = {'foo': 'bar'}; - detector..watch(map, null, null)..collectChanges(); - - map['foo'] = 'b' + 'ar'; - - expect(detector.collectChanges().moveNext()).toEqual(false); - }); - - it('should not see a NaN value as a change', () { - var map = {'foo': double.NAN}; - var record = detector..watch(map, null, null)..collectChanges(); - - expect(detector.collectChanges().moveNext()).toEqual(false); - }); + addMapSpec(useObservable: false); + addMapSpec(useObservable: true); }); describe('function watching', () { @@ -806,6 +678,453 @@ void main() { })); } +void addListSpec({bool useObservable}) { + Function wrap(fn) => useObservable ? async(fn) : sync(fn); + + List listFactory(List list) => useObservable ? new ObservableList.from(list) : list; + + Iterator getChangeIterator() { + if (useObservable) microLeap(); + return detector.collectChanges(); + } + + describe('use observable: $useObservable', () { + describe('previous state', () { + it('should store on addition', wrap(() { + var list = listFactory([]); + var record = detector.watch(list, null, null); + expect(getChangeIterator().moveNext()).toEqual(false); + var iterator; + + list.add('a'); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a[null -> 0]'], + previous: [], + additions: ['a[null -> 0]'], + moves: [], + removals: [])); + + list.add('b'); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a', 'b[null -> 1]'], + previous: ['a'], + additions: ['b[null -> 1]'], + moves: [], + removals: [])); + })); + + xit('should handle swapping elements correctly', wrap(() { + var list = listFactory([1, 2]); + print(list.runtimeType); + detector.watch(list, null, null); + getChangeIterator().moveNext(); + var iterator; + + // reverse the list. + list.setAll(0, list.reversed.toList()); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['2[1 -> 0]', '1[0 -> 1]'], + previous: ['1[0 -> 1]', '2[1 -> 0]'], + additions: [], + moves: ['2[1 -> 0]', '1[0 -> 1]'], + removals: [])); + })); + + it('should handle swapping elements correctly - gh1097', wrap(() { + // This test would only have failed in non-checked mode only + var list = listFactory(['a', 'b', 'c']); + var record = detector.watch(list, null, null); + var iterator = getChangeIterator()..moveNext(); + + list..clear()..addAll(['b', 'a', 'c']); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['b[1 -> 0]', 'a[0 -> 1]', 'c'], + previous: ['a[0 -> 1]', 'b[1 -> 0]', 'c'], + additions: [], + moves: ['b[1 -> 0]', 'a[0 -> 1]'], + removals: [])); + + list..clear()..addAll(['b', 'c', 'a']); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['b', 'c[2 -> 1]', 'a[1 -> 2]'], + previous: ['b', 'a[1 -> 2]', 'c[2 -> 1]'], + additions: [], + moves: ['c[2 -> 1]', 'a[1 -> 2]'], + removals: [])); + })); + }); + + it('should detect changes in list', wrap(() { + var list = listFactory([]); + var record = detector.watch(list, null, 'handler'); + expect(getChangeIterator().moveNext()).toEqual(false); + var iterator; + + list.add('a'); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a[null -> 0]'], + additions: ['a[null -> 0]'], + moves: [], + removals: [])); + + list.add('b'); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a', 'b[null -> 1]'], + previous: ['a'], + additions: ['b[null -> 1]'], + moves: [], + removals: [])); + + list.add('c'); + list.add('d'); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a', 'b', 'c[null -> 2]', 'd[null -> 3]'], + previous: ['a', 'b'], + additions: ['c[null -> 2]', 'd[null -> 3]'], + moves: [], + removals: [])); + + list.remove('c'); + expect(list).toEqual(['a', 'b', 'd']); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a', 'b', 'd[3 -> 2]'], + previous: ['a', 'b', 'c[2 -> null]', 'd[3 -> 2]'], + additions: [], + moves: ['d[3 -> 2]'], + removals: ['c[2 -> null]'])); + + list.clear(); + list.addAll(['d', 'c', 'b', 'a']); + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['d[2 -> 0]', 'c[null -> 1]', 'b[1 -> 2]', 'a[0 -> 3]'], + previous: ['a[0 -> 3]', 'b[1 -> 2]', 'd[2 -> 0]'], + additions: ['c[null -> 1]'], + moves: ['d[2 -> 0]', 'b[1 -> 2]', 'a[0 -> 3]'], + removals: [])); + })); + + it('should test string by value rather than by reference', wrap(() { + var list = listFactory(['a', 'boo']); + detector.watch(list, null, null); + getChangeIterator(); + + list[1] = 'b' + 'oo'; + + expect(getChangeIterator().moveNext()).toEqual(false); + })); + + it('should ignore [NaN] != [NaN]', wrap(() { + var list = listFactory([double.NAN]); + var record = detector.watch(list, null, null); + getChangeIterator(); + + expect(getChangeIterator().moveNext()).toEqual(false); + })); + + it('should remove and add same item', wrap(() { + var list = listFactory(['a', 'b', 'c']); + var record = detector.watch(list, null, 'handler'); + var iterator; + getChangeIterator(); + + list.remove('b'); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a', 'c[2 -> 1]'], + previous: ['a', 'b[1 -> null]', 'c[2 -> 1]'], + additions: [], + moves: ['c[2 -> 1]'], + removals: ['b[1 -> null]'])); + + list.insert(1, 'b'); + expect(list).toEqual(['a', 'b', 'c']); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a', 'b[null -> 1]', 'c[1 -> 2]'], + previous: ['a', 'c[1 -> 2]'], + additions: ['b[null -> 1]'], + moves: ['c[1 -> 2]'], + removals: [])); + })); + + it('should support duplicates', wrap(() { + var list = listFactory(['a', 'a', 'a', 'b', 'b']); + var record = detector.watch(list, null, 'handler'); + getChangeIterator(); + + list.removeAt(0); + var iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['a', 'a', 'b[3 -> 2]', 'b[4 -> 3]'], + previous: ['a', 'a', 'a[2 -> null]', 'b[3 -> 2]', 'b[4 -> 3]'], + additions: [], + moves: ['b[3 -> 2]', 'b[4 -> 3]'], + removals: ['a[2 -> null]'])); + })); + + + it('should support insertions/moves', wrap(() { + var list = listFactory(['a', 'a', 'b', 'b']); + var record = detector.watch(list, null, 'handler'); + var iterator; + getChangeIterator(); + list.insert(0, 'b'); + expect(list).toEqual(['b', 'a', 'a', 'b', 'b']); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['b[2 -> 0]', 'a[0 -> 1]', 'a[1 -> 2]', 'b', 'b[null -> 4]'], + previous: ['a[0 -> 1]', 'a[1 -> 2]', 'b[2 -> 0]', 'b'], + additions: ['b[null -> 4]'], + moves: ['b[2 -> 0]', 'a[0 -> 1]', 'a[1 -> 2]'], + removals: [])); + })); + + it('should bug', wrap(() { + var list = listFactory([1, 2, 3, 4]); + var record = detector.watch(list, null, 'handler'); + var iterator; + + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['1[null -> 0]', '2[null -> 1]', '3[null -> 2]', '4[null -> 3]'], + additions: ['1[null -> 0]', '2[null -> 1]', '3[null -> 2]', '4[null -> 3]'], + moves: [], + removals: [])); + detector.collectChanges(); + + list.removeRange(0, 1); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['2[1 -> 0]', '3[2 -> 1]', '4[3 -> 2]'], + previous: ['1[0 -> null]', '2[1 -> 0]', '3[2 -> 1]', '4[3 -> 2]'], + additions: [], + moves: ['2[1 -> 0]', '3[2 -> 1]', '4[3 -> 2]'], + removals: ['1[0 -> null]'])); + + list.insert(0, 1); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['1[null -> 0]', '2[0 -> 1]', '3[1 -> 2]', '4[2 -> 3]'], + previous: ['2[0 -> 1]', '3[1 -> 2]', '4[2 -> 3]'], + additions: ['1[null -> 0]'], + moves: ['2[0 -> 1]', '3[1 -> 2]', '4[2 -> 3]'], + removals: [])); + })); + + it('should properly support objects with equality', wrap(() { + FooBar.fooIds = 0; + var list = listFactory([new FooBar('a', 'a'), new FooBar('a', 'a')]); + var record = detector.watch(list, null, 'handler'); + var iterator; + + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['(0)a-a[null -> 0]', '(1)a-a[null -> 1]'], + additions: ['(0)a-a[null -> 0]', '(1)a-a[null -> 1]'], + moves: [], + removals: [])); + detector.collectChanges(); + + list.removeRange(0, 1); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['(1)a-a[1 -> 0]'], + previous: ['(0)a-a[0 -> null]', '(1)a-a[1 -> 0]'], + additions: [], + moves: ['(1)a-a[1 -> 0]'], + removals: ['(0)a-a[0 -> null]'])); + + list.insert(0, new FooBar('a', 'a')); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['(2)a-a[null -> 0]', '(1)a-a[0 -> 1]'], + previous: ['(1)a-a[0 -> 1]'], + additions: ['(2)a-a[null -> 0]'], + moves: ['(1)a-a[0 -> 1]'], + removals: [])); + })); + + it('should not report unnecessary moves', wrap(() { + var list = listFactory(['a', 'b', 'c']); + var record = detector.watch(list, null, null); + var iterator = detector.collectChanges()..moveNext(); + + list..clear()..addAll(['b', 'a', 'c']); + iterator = getChangeIterator()..moveNext(); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['b[1 -> 0]', 'a[0 -> 1]', 'c'], + previous: ['a[0 -> 1]', 'b[1 -> 0]', 'c'], + additions: [], + moves: ['b[1 -> 0]', 'a[0 -> 1]'], + removals: [])); + })); + }); +} + +void addMapSpec({bool useObservable}) { + Function wrap(fn) => useObservable ? async(fn) : sync(fn); + + Map mapFactory(Map map) => useObservable ? new ObservableMap.from(map) : map; + + Iterator getChangeIterator() { + if (useObservable) microLeap(); + return detector.collectChanges(); + } + + describe('use observable: $useObservable', () { + describe('previous state', () { + it('should store on insertion', wrap(() { + var map = mapFactory({}); + var record = detector.watch(map, null, null); + expect(getChangeIterator().moveNext()).toEqual(false); + var iterator; + + map['a'] = 1; + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualMapRecord( + map: ['a[null -> 1]'], + previous: [], + additions: ['a[null -> 1]'], + changes: [], + removals: [])); + + map['b'] = 2; + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualMapRecord( + map: ['a', 'b[null -> 2]'], + previous: ['a'], + additions: ['b[null -> 2]'], + changes: [], + removals: [])); + })); + + it('should handle changing key/values correctly', wrap(() { + var map = mapFactory({1: 10, 2: 20}); + var record = detector.watch(map, null, null); + getChangeIterator().moveNext(); + var iterator; + + map[1] = 20; + map[2] = 10; + iterator = getChangeIterator(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualMapRecord( + map: ['1[10 -> 20]', '2[20 -> 10]'], + previous: ['1[10 -> 20]', '2[20 -> 10]'], + additions: [], + changes: ['1[10 -> 20]', '2[20 -> 10]'], + removals: [])); + })); + }); + + it('should do basic map watching', wrap(() { + var map = mapFactory({}); + var record = detector.watch(map, null, 'handler'); + expect(getChangeIterator().moveNext()).toEqual(false); + + var changeIterator; + map['a'] = 'A'; + changeIterator = getChangeIterator(); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue, toEqualMapRecord( + map: ['a[null -> A]'], + previous: [], + additions: ['a[null -> A]'], + changes: [], + removals: [])); + + map['b'] = 'B'; + changeIterator = getChangeIterator(); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue, toEqualMapRecord( + map: ['a', 'b[null -> B]'], + previous: ['a'], + additions: ['b[null -> B]'], + changes: [], + removals: [])); + + map['b'] = 'BB'; + map['d'] = 'D'; + changeIterator = getChangeIterator(); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue, toEqualMapRecord( + map: ['a', 'b[B -> BB]', 'd[null -> D]'], + previous: ['a', 'b[B -> BB]'], + additions: ['d[null -> D]'], + changes: ['b[B -> BB]'], + removals: [])); + + map.remove('b'); + expect(map).toEqual({'a': 'A', 'd':'D'}); + changeIterator = getChangeIterator(); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue, toEqualMapRecord( + map: ['a', 'd'], + previous: ['a', 'b[BB -> null]', 'd'], + additions: [], + changes: [], + removals: ['b[BB -> null]'])); + + map.clear(); + changeIterator = getChangeIterator(); + expect(changeIterator.moveNext()).toEqual(true); + expect(changeIterator.current.currentValue, toEqualMapRecord( + map: [], + previous: ['a[A -> null]', 'd[D -> null]'], + additions: [], + changes: [], + removals: ['a[A -> null]', 'd[D -> null]'])); + })); + + it('should test string keys by value rather than by reference', wrap(() { + var map = mapFactory({'foo': 0}); + detector.watch(map, null, null); + getChangeIterator(); + + map['f' + 'oo'] = 0; + + expect(getChangeIterator().moveNext()).toEqual(false); + })); + + it('should test string values by value rather than by reference', wrap(() { + var map = mapFactory({'foo': 'bar'}); + detector.watch(map, null, null); + getChangeIterator(); + + map['foo'] = 'b' + 'ar'; + + expect(getChangeIterator().moveNext()).toEqual(false); + })); + + it('should not see a NaN value as a change', wrap(() { + var map = mapFactory({'foo': double.NAN}); + detector.watch(map, null, null); + getChangeIterator(); + + expect(getChangeIterator().moveNext()).toEqual(false); + })); + }); +} class _User { String first; @@ -824,12 +1143,33 @@ class _User { } } +class _ObservableUser extends ChangeNotifier { + String __$first; + String get first => __$first; + void set first(String value) { + __$first = notifyPropertyChange(#first, __$first, value); + } + + String __$last; + String get last => __$last; + void set last(String value) { + __$last = notifyPropertyChange(#first, __$last, value); + } + + _ObservableUser([String first, String last]) { + this.first = first; + this.last = last; + } +} + Matcher toEqualCollectionRecord({collection, previous, additions, moves, removals}) => new CollectionRecordMatcher(collection:collection, previous: previous, additions:additions, moves:moves, removals:removals); + Matcher toEqualMapRecord({map, previous, additions, changes, removals}) => new MapRecordMatcher(map:map, previous: previous, additions:additions, changes:changes, removals:removals); + Matcher toEqualChanges(List changes) => new ChangeMatcher(changes); class ChangeMatcher extends Matcher {