Deprecate Observable, add ChangeNotifier, setup travis (#11)

* Deprecate Observable, setup Travis

* Make changes suggested via review

* Update .travis.yml

Remove stable branch, as `collection` dependency won't resolve on it.

Author: matanlurey

.travis.yml

@@ -0,0 +1,8 @@
+language: dart
+
+dart:
+  - dev
+  # Currently requires a dev-branch in order to use properly.
+  # - stable
+
+script: ./tool/presubmit.sh

CHANGELOG.md

@@ -1,3 +1,36 @@
+## 0.17.0
+
+This is a larger change with a goal of no runtime changes for current
+customers, but in the future `Observable` will [become][issue_10] a very
+lightweight interface, i.e.:
+
+```dart
+abstract class Observable<C extends ChangeRecord> {
+  Stream<List<C>> get changes;
+}
+```
+
+[issue_10]: https://github.com/dart-lang/observable/issues/10
+
+* Started deprecating the wide `Observable` interface
+    * `ChangeNotifier` should be used as a base class for these methods:
+        * `Observable.observed`
+        * `Observable.unobserved`
+        * `Observable.hasObservers`
+        * `Observable.deliverChanges`
+        * `Observable.notifyChange`
+    * `PropertyChangeNotifier` should be used for these methods:
+        * `Observable.notifyPropertyChange`
+    * Temporarily, `Observable` _uses_ `ChangeNotifier`
+        * Existing users of anything but `implements Observable` should
+          move to implementing or extending `ChangeNotifier`. In a
+          future release `Observable` will reduce API surface down to
+          an abstract `Stream<List<C>> get changes`.
+* Added the `ChangeNotifier` and `PropertyChangeNotifier` classes
+    * Can be used to implement `Observable` in a generic manner
+* Observable is now `Observable<C extends ChangeRecord>`
+    * When passing a generic type `C`, `notifyPropertyChange` is illegal
+
 ## 0.16.0
 
 * Refactored `MapChangeRecord`

lib/observable.dart

@@ -4,10 +4,11 @@
 
 library observable;
 
+export 'src/change_notifier.dart' show ChangeNotifier, PropertyChangeNotifier;
 export 'src/differs.dart' show Differ, EqualityDiffer, ListDiffer, MapDiffer;
-export 'src/records.dart' show ChangeRecord, ListChangeRecord, MapChangeRecord;
+export 'src/records.dart'
+    show ChangeRecord, ListChangeRecord, MapChangeRecord, PropertyChangeRecord;
 export 'src/observable.dart';
 export 'src/observable_list.dart';
 export 'src/observable_map.dart';
-export 'src/property_change_record.dart';
 export 'src/to_observable.dart';

lib/src/change_notifier.dart

@@ -0,0 +1,144 @@
+import 'dart:async';
+
+import 'package:meta/meta.dart';
+
+import 'internal.dart';
+import 'observable.dart';
+import 'records.dart';
+
+/// Supplies [changes] and various hooks to implement [Observable].
+///
+/// May use [notifyChange] to queue a change record; they are asynchronously
+/// delivered at the end of the VM turn.
+///
+/// [ChangeNotifier] may be extended, mixed in, or used as a delegate.
+class ChangeNotifier<C extends ChangeRecord> implements Observable<C> {
+  StreamController<List<C>> _changes;
+
+  bool _scheduled = false;
+  List<C> _queue;
+
+  /// Emits a list of changes when the state of the object changes.
+  ///
+  /// Changes should produced in order, if significant.
+  @override
+  Stream<List<C>> get changes {
+    return (_changes ??= new StreamController<List<C>>.broadcast(
+      sync: true,
+      onListen: observed,
+      onCancel: unobserved,
+    ))
+        .stream;
+  }
+
+  /// May override to be notified when [changes] is first observed.
+  @override
+  @protected
+  @mustCallSuper
+  void observed() {}
+
+  /// May override to be notified when [changes] is no longer observed.
+  @override
+  @protected
+  @mustCallSuper
+  void unobserved() {
+    _changes = _queue = null;
+  }
+
+  /// If [hasObservers], synchronously emits [changes] that have been queued.
+  ///
+  /// Returns `true` if changes were emitted.
+  @override
+  @protected
+  @mustCallSuper
+  bool deliverChanges() {
+    List<ChangeRecord> changes;
+    if (_scheduled && hasObservers) {
+      if (_queue != null) {
+        changes = freezeInDevMode(_queue);
+        _queue = null;
+      } else {
+        changes = ChangeRecord.ANY;
+      }
+      _scheduled = false;
+      _changes.add(changes);
+    }
+    return changes != null;
+  }
+
+  /// Whether [changes] has at least one active listener.
+  ///
+  /// May be used to optimize whether to produce change records.
+  @override
+  bool get hasObservers => _changes?.hasListener == true;
+
+  /// Schedules [change] to be delivered.
+  ///
+  /// If [change] is omitted then [ChangeRecord.ANY] will be sent.
+  ///
+  /// If there are no listeners to [changes], this method does nothing.
+  @override
+  void notifyChange([C change]) {
+    if (!hasObservers) {
+      return;
+    }
+    if (change != null) {
+      (_queue ??= <C>[]).add(change);
+    }
+    if (!_scheduled) {
+      scheduleMicrotask(deliverChanges);
+      _scheduled = true;
+    }
+  }
+
+  @Deprecated('Exists to make migrations off Observable easier')
+  @override
+  @protected
+  /*=T*/ notifyPropertyChange/*<T>*/(
+    Symbol field,
+    /*=T*/
+    oldValue,
+    /*=T*/
+    newValue,
+  ) {
+    throw new UnsupportedError('Not supported by ChangeNotifier');
+  }
+}
+
+/// Implements [notifyPropertyChange] for classes that need support.
+///
+/// Will be folded entirely into [PropertyChangeNotifier] in the future.
+@Deprecated('Exists to make migrations off Observable easier')
+abstract class PropertyChangeMixin implements ChangeNotifier {
+  @override
+  /*=T*/ notifyPropertyChange/*<T>*/(
+    Symbol field,
+    /*=T*/
+    oldValue,
+    /*=T*/
+    newValue,
+  ) {
+    if (hasObservers && oldValue != newValue) {
+      notifyChange(
+        new PropertyChangeRecord/*<T>*/(
+          this,
+          field,
+          oldValue,
+          newValue,
+        ),
+      );
+    }
+    return newValue;
+  }
+}
+
+/// Supplies property `changes` and various hooks to implement [Observable].
+///
+/// May use `notifyChange` or `notifyPropertyChange` to queue a property change
+/// record; they are asynchronously delivered at the end of the VM turn.
+///
+/// [PropertyChangeNotifier] may be extended or used as a delegate. To use as
+/// a mixin, instead use with [PropertyChangeMixin]:
+///     with ChangeNotifier<PropertyChangeRecord>, PropertyChangeMixin
+class PropertyChangeNotifier = ChangeNotifier<PropertyChangeRecord>
+    with PropertyChangeMixin;

lib/src/differs.dart

@@ -17,7 +17,7 @@ part 'differs/map_differ.dart';
 
 /// Generic comparisons between two comparable objects.
 abstract class Differ<E> {
-  /// Returns a list of change records between [e1] and [e2].
+  /// Returns a list of change records between [oldValue] and [newValue].
   ///
   /// A return value of an empty [ChangeRecord.NONE] means no changes found.
   List<ChangeRecord> diff(E oldValue, E newValue);

lib/src/differs/list_differ.dart

@@ -210,7 +210,7 @@ List<ListChangeRecord/*<E>*/ > _calcSplices/*<E>*/(
   }
 
   if (currentStart == currentEnd) {
-    final spliceRemoved = old.sublist(oldStart, oldStart + (oldEnd - oldStart));
+    final spliceRemoved = old.sublist(oldStart, oldEnd);
     return [
       new ListChangeRecord/*<E>*/ .remove(
         current,

lib/src/observable.dart

@@ -5,102 +5,103 @@
 library observable.src.observable;
 
 import 'dart:async';
-import 'dart:collection' show UnmodifiableListView;
 
 import 'package:meta/meta.dart';
 
-import 'property_change_record.dart' show PropertyChangeRecord;
-import 'records.dart' show ChangeRecord;
+import 'change_notifier.dart';
+import 'records.dart';
 
-/// Represents an object with observable properties. This is used by data in
-/// model-view architectures to notify interested parties of [changes] to the
-/// object's properties (fields or getter/setter pairs).
+/// Represents an object with observable state or properties.
 ///
 /// The interface does not require any specific technique to implement
-/// observability. You can implement it in the following ways:
-///
-/// - Deriving from this class via a mixin or base class. When a field,
-///   property, or indexable item is changed, the derived class should call
-///   [notifyPropertyChange]. See that method for an example.
-/// - Implementing this interface and providing your own implementation.
-abstract class Observable {
-  StreamController<List<ChangeRecord>> _changes;
+/// observability. You may implement it in the following ways:
+/// - Extend or mixin [ChangeNotifier]
+/// - Implement the interface yourself and provide your own implementation
+abstract class Observable<C extends ChangeRecord> {
+  // To be removed when https://github.com/dart-lang/observable/issues/10
+  final ChangeNotifier<C> _delegate = new ChangeNotifier<C>();
 
-  List<ChangeRecord> _records;
+  // Whether Observable was not given a type.
+  final bool _isNotGeneric = C == dynamic;
 
-  /// The stream of property change records to this object, delivered
-  /// asynchronously.
+  /// Emits a list of changes when the state of the object changes.
   ///
-  /// [deliverChanges] can be called to force synchronous delivery.
-  Stream<List<ChangeRecord>> get changes {
-    if (_changes == null) {
-      _changes = new StreamController.broadcast(
-          sync: true, onListen: observed, onCancel: unobserved);
-    }
-    return _changes.stream;
-  }
+  /// Changes should produced in order, if significant.
+  Stream<List<C>> get changes => _delegate.changes;
 
-  /// Derived classes may override this method to be called when the [changes]
-  /// are first observed.
-  // TODO(tvolkert): @mustCallSuper (github.com/dart-lang/sdk/issues/27275)
+  /// May override to be notified when [changes] is first observed.
   @protected
-  void observed() {}
+  @mustCallSuper
+  @Deprecated('Use ChangeNotifier instead to have this method available')
+  // REMOVE IGNORE when https://github.com/dart-lang/observable/issues/10
+  // ignore: invalid_use_of_protected_member
+  void observed() => _delegate.observed();
 
-  /// Derived classes may override this method to be called when the [changes]
-  /// are no longer being observed.
-  // TODO(tvolkert): @mustCallSuper (github.com/dart-lang/sdk/issues/27275)
+  /// May override to be notified when [changes] is no longer observed.
   @protected
-  void unobserved() {
-    // Free some memory
-    _changes = null;
-  }
+  @mustCallSuper
+  @Deprecated('Use ChangeNotifier instead to have this method available')
+  // REMOVE IGNORE when https://github.com/dart-lang/observable/issues/10
+  // ignore: invalid_use_of_protected_member
+  void unobserved() => _delegate.unobserved();
 
   /// True if this object has any observers.
-  bool get hasObservers => _changes != null && _changes.hasListener;
+  @Deprecated('Use ChangeNotifier instead to have this method available')
+  bool get hasObservers => _delegate.hasObservers;
 
-  /// Synchronously deliver pending [changes].
+  /// If [hasObservers], synchronously emits [changes] that have been queued.
   ///
-  /// Returns `true` if any records were delivered, otherwise `false`.
-  /// Pending records will be cleared regardless, to keep newly added
-  /// observers from being notified of changes that occurred before
-  /// they started observing.
-  bool deliverChanges() {
-    List<ChangeRecord> records = _records;
-    _records = null;
-    if (hasObservers && records != null) {
-      _changes.add(new UnmodifiableListView<ChangeRecord>(records));
-      return true;
-    }
-    return false;
-  }
+  /// Returns `true` if changes were emitted.
+  @Deprecated('Use ChangeNotifier instead to have this method available')
+  // REMOVE IGNORE when https://github.com/dart-lang/observable/issues/10
+  // ignore: invalid_use_of_protected_member
+  bool deliverChanges() => _delegate.deliverChanges();
 
   /// Notify that the [field] name of this object has been changed.
   ///
   /// The [oldValue] and [newValue] are also recorded. If the two values are
   /// equal, no change will be recorded.
   ///
   /// For convenience this returns [newValue].
+  ///
+  /// ## Deprecated
+  ///
+  /// All [Observable] objects will no longer be required to emit change records
+  /// when any property changes. For example, `ObservableList` will only emit
+  /// on `ObservableList.changes`, instead of on `ObservableList.listChanges`.
+  ///
+  /// If you are using a typed `implements/extends Observable<C>`, it is illegal
+  /// to call this method - will throw an [UnsupportedError] when called.
+  @Deprecated('Use PropertyChangeNotifier')
   /*=T*/ notifyPropertyChange/*<T>*/(
-      Symbol field, /*=T*/ oldValue, /*=T*/ newValue) {
+    Symbol field,
+    /*=T*/
+    oldValue,
+    /*=T*/
+    newValue,
+  ) {
     if (hasObservers && oldValue != newValue) {
-      notifyChange(new PropertyChangeRecord(this, field, oldValue, newValue));
+      if (_isNotGeneric) {
+        notifyChange(
+          new PropertyChangeRecord(
+            this,
+            field,
+            oldValue,
+            newValue,
+          ) as C,
+        );
+      } else {
+        throw new UnsupportedError('Generic typed Observable does not support');
+      }
     }
     return newValue;
   }
 
-  /// Notify observers of a change.
+  /// Schedules [change] to be delivered.
   ///
-  /// This will automatically schedule [deliverChanges].
+  /// If [change] is omitted then [ChangeRecord.ANY] will be sent.
   ///
-  /// For most objects [Observable.notifyPropertyChange] is more convenient, but
-  /// collections sometimes deliver other types of changes such as a
-  /// [MapChangeRecord].
-  void notifyChange(ChangeRecord record) {
-    if (!hasObservers) return;
-    if (_records == null) {
-      _records = [];
-      scheduleMicrotask(deliverChanges);
-    }
-    _records.add(record);
-  }
+  /// If there are no listeners to [changes], this method does nothing.
+  @Deprecated('Use ChangeNotifier instead to have this method available')
+  void notifyChange([C change]) => _delegate.notifyChange(change);
 }