Avoid unnecessary observable notifications of Iterable or Map fields (#…

…951)

mobx/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.2.3
+
+- Avoid unnecessary observable notifications of `@observable` `Iterable` or `Map` fields of Stores by [@amondnet](https://github.com/amondnet)  in [#951](https://github.com/mobxjs/mobx.dart/pull/951)
+
 ## 2.2.2
 
 - Fix [#956]((https://github.com/mobxjs/mobx.dart/issues/956)): ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately. by [@amondnet](https://github.com/amondnet) in [#962](https://github.com/mobxjs/mobx.dart/pull/962)

mobx/lib/src/core/atom_extensions.dart

@@ -1,5 +1,7 @@
 import 'package:mobx/mobx.dart';
 
+import '../utils.dart';
+
 extension AtomSpyReporter on Atom {
   void reportRead() {
     context.enforceReadPolicy(this);
@@ -8,11 +10,10 @@ extension AtomSpyReporter on Atom {
 
   void reportWrite<T>(T newValue, T oldValue, void Function() setNewValue,
       {EqualityComparer<T>? equals}) {
-    final areEqual =
-        equals == null ? oldValue == newValue : equals(oldValue, newValue);
+    final areEqual = equals ?? equatable;
 
     // Avoid unnecessary observable notifications of @observable fields of Stores
-    if (areEqual) {
+    if (areEqual(newValue, oldValue)) {
       return;
     }
 

mobx/lib/src/core/observable.dart

@@ -105,7 +105,7 @@ class Observable<T> extends Atom
     }
 
     final areEqual =
-        equals == null ? prepared == value : equals!(prepared, _value);
+        equals == null ? equatable(prepared, value) : equals!(prepared, _value);
 
     return (!areEqual) ? prepared : WillChangeNotification.unchanged;
   }

mobx/lib/src/utils.dart

@@ -1,5 +1,7 @@
 import 'dart:async';
 
+import 'package:collection/collection.dart' show DeepCollectionEquality;
+
 const Duration ms = Duration(milliseconds: 1);
 
 Timer Function(void Function()) createDelayedScheduler(int delayMs) =>
@@ -20,3 +22,19 @@ mixin DebugCreationStack {
     return result;
   }();
 }
+
+/// Determines whether [a] and [b] are equal.
+bool equatable<T>(T a, T b) {
+  if (identical(a, b)) return true;
+  if (a is Iterable || a is Map) {
+    if (!_equality.equals(a, b)) return false;
+  } else if (a.runtimeType != b.runtimeType) {
+    return false;
+  } else if (a != b) {
+    return false;
+  }
+
+  return true;
+}
+
+const DeepCollectionEquality _equality = DeepCollectionEquality();

mobx/lib/version.dart

@@ -1,4 +1,4 @@
 // Generated via set_version.dart. !!!DO NOT MODIFY BY HAND!!!
 
 /// The current version as per `pubspec.yaml`.
-const version = '2.2.2';
+const version = '2.2.3';

mobx/pubspec.yaml

@@ -1,5 +1,5 @@
 name: mobx
-version: 2.2.2
+version: 2.2.3
 description: "MobX is a library for reactively managing the state of your applications. Use the power of observables, actions, and reactions to supercharge your Dart and Flutter apps."
 
 homepage: https://github.com/mobxjs/mobx.dart
@@ -10,10 +10,10 @@ environment:
 
 dependencies:
   meta: ^1.3.0
+  collection: ^1.15.0
 
 dev_dependencies:
   build_runner: ^2.0.6
-  collection: ^1.15.0
   coverage: ^1.0.1
   fake_async: ^1.2.0
   lints: ^2.0.0

mobx/test/atom_extensions_test.dart

@@ -4,75 +4,123 @@ import 'package:test/test.dart';
 void main() {
   test(
       'when write to @observable field with changed value, should trigger notifications for downstream',
-      () {
-    final store = _ExampleStore();
+          () {
+        final store = _ExampleStore();
 
-    final autorunResults = <String>[];
-    autorun((_) => autorunResults.add(store.value));
+        final autorunResults = <String>[];
+        autorun((_) => autorunResults.add(store.value));
 
-    expect(autorunResults, ['first']);
+        expect(autorunResults, ['first']);
 
-    store.value = 'second';
+        store.value = 'second';
 
-    expect(autorunResults, ['first', 'second']);
-  });
+        expect(autorunResults, ['first', 'second']);
+      });
 
   // fixed by #855
   test(
       'when write to @observable field with unchanged value, should not trigger notifications for downstream',
-      () {
-    final store = _ExampleStore();
+          () {
+        final store = _ExampleStore();
 
-    final autorunResults = <String>[];
-    autorun((_) => autorunResults.add(store.value));
+        final autorunResults = <String>[];
+        autorun((_) => autorunResults.add(store.value));
 
-    expect(autorunResults, ['first']);
+        expect(autorunResults, ['first']);
 
-    store.value = store.value;
+        store.value = store.value;
 
-    expect(autorunResults, ['first']);
-  });
+        expect(autorunResults, ['first']);
+      });
 
   test(
       'when write to @alwaysNotify field with unchanged value, should trigger notifications for downstream',
-      () {
-    final store = _ExampleStore();
+          () {
+        final store = _ExampleStore();
 
-    final autorunResults = <String>[];
-    autorun((_) => autorunResults.add(store.value2));
+        final autorunResults = <String>[];
+        autorun((_) => autorunResults.add(store.value2));
 
-    expect(autorunResults, ['first']);
+        expect(autorunResults, ['first']);
 
-    store.value2 = store.value2;
+        store.value2 = store.value2;
 
-    expect(autorunResults, ['first', 'first']);
-  });
+        expect(autorunResults, ['first', 'first']);
+      });
 
   test(
       'when write to @MakeObservable(equals: "a?.length == b?.length") field with changed value and not equals, should trigger notifications for downstream',
-      () {
-    final store = _ExampleStore();
+          () {
+        final store = _ExampleStore();
 
-    final autorunResults = <String>[];
-    autorun((_) => autorunResults.add(store.value3));
+        final autorunResults = <int>[];
+        autorun((_) => autorunResults.add(store.value3.length));
 
-    expect(autorunResults, ['first']); // length: 5
+        expect(autorunResults, [5]); // length: 5
 
-    // length: 5, should not trigger
-    store.value3 = 'third';
+        // length: 5, should not trigger
+        store.value3 = 'third';
 
-    expect(autorunResults, ['first']);
+        expect(autorunResults, [5]);
 
-    // length: 6, should trigger
-    store.value3 = 'second';
+        // length: 6, should trigger
+        store.value3 = 'second';
 
-    expect(autorunResults, ['first', 'second']);
-  });
+        expect(autorunResults, [5, 6]);
+      });
+
+  test(
+      'when write to iterable @observable field with unchanged value, should not trigger notifications for downstream',
+          () {
+        final store = _ExampleStore();
+
+        final autorunResults = <List<String>>[];
+        autorun((_) => autorunResults.add(store.list));
+
+        store.list = ['first'];
+        expect(autorunResults, [
+          ['first']
+        ]);
+
+        store.list = ['first'];
+        expect(autorunResults, [
+          ['first']
+        ]);
+
+        store.list = ['first'];
+        expect(autorunResults, [
+          ['first']
+        ]);
+      });
+
+  test(
+      'when write to map @observable field with unchanged value, should not trigger notifications for downstream',
+          () {
+        final store = _ExampleStore();
+
+        final autorunResults = <Map<String, int>>[];
+        autorun((_) => autorunResults.add(store.map));
+
+        store.map = {'first': 1};
+        expect(autorunResults, [
+          {'first': 1}
+        ]);
+
+        store.map = {'first': 1};
+        expect(autorunResults, [
+          {'first': 1}
+        ]);
+
+        store.map = {'first': 1};
+        expect(autorunResults, [
+          {'first': 1}
+        ]);
+      });
 }
 
 class _ExampleStore = __ExampleStore with _$_ExampleStore;
 
-bool _equals(String? oldValue, String? newValue) => (oldValue == newValue);
+bool _equals(String? oldValue, String? newValue) => (oldValue?.length == newValue?.length);
 
 abstract class __ExampleStore with Store {
   @observable
@@ -83,6 +131,12 @@ abstract class __ExampleStore with Store {
 
   @MakeObservable(equals: _equals)
   String value3 = 'first';
+
+  @observable
+  List<String> list = ['first'];
+
+  @observable
+  Map<String, int> map = {'first': 1};
 }
 
 // This is what typically a mobx codegen will generate.
@@ -105,7 +159,7 @@ mixin _$_ExampleStore on __ExampleStore, Store {
 
   // ignore: non_constant_identifier_names
   late final _$value2Atom =
-      Atom(name: '__ExampleStore.value2', context: context);
+  Atom(name: '__ExampleStore.value2', context: context);
 
   @override
   String get value2 {
@@ -122,7 +176,7 @@ mixin _$_ExampleStore on __ExampleStore, Store {
 
   // ignore: non_constant_identifier_names
   late final _$value3Atom =
-      Atom(name: '__ExampleStore.value3', context: context);
+  Atom(name: '__ExampleStore.value3', context: context);
 
   @override
   String get value3 {
@@ -135,7 +189,38 @@ mixin _$_ExampleStore on __ExampleStore, Store {
     _$value3Atom.reportWrite(value, super.value3, () {
       super.value3 = value;
     },
-        equals: (String? oldValue, String? newValue) =>
-            oldValue?.length == newValue?.length);
+        equals: _equals);
+  }
+
+  // ignore: non_constant_identifier_names
+  late final _$listAtom = Atom(name: '__ExampleStore.list', context: context);
+
+  @override
+  List<String> get list {
+    _$listAtom.reportRead();
+    return super.list;
+  }
+
+  @override
+  set list(List<String> value) {
+    _$listAtom.reportWrite(value, super.list, () {
+      super.list = value;
+    });
+  }
+
+  // ignore: non_constant_identifier_names
+  late final _$mapAtom = Atom(name: '__ExampleStore.map', context: context);
+
+  @override
+  Map<String, int> get map {
+    _$mapAtom.reportRead();
+    return super.map;
+  }
+
+  @override
+  set map(Map<String, int> value) {
+    _$mapAtom.reportWrite(value, super.map, () {
+      super.map = value;
+    });
   }
 }