🐛 Fixes issue in ObservableList while using Iterables.

mobxjs · Oct 17, 2023 · e951de2 · e951de2
1 parent 4539e69
commit e951de2
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 17 deletions.
diff --git a/mobx/CHANGELOG.md b/mobx/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 2.2.1
+
+- Reduces unnecessary iterations while using `Iterables` with `addAll`, `insertAll`, `replaceRange`, `setAll` and `setRange` methods. [#942](https://github.com/mobxjs/mobx.dart/pull/942).
+
 ## 2.2.0
 
 - Allows a reaction to be fired even if the value hasn't changed by [@amondnet](https://github.com/amondnet) in [#907](https://github.com/mobxjs/mobx.dart/pull/907)

diff --git a/mobx/lib/src/api/observable_collections/observable_list.dart b/mobx/lib/src/api/observable_collections/observable_list.dart
@@ -115,10 +115,13 @@ class ObservableList<T>
   @override
   void addAll(Iterable<T> iterable) {
     _context.conditionallyRunInAction(() {
-      if (iterable.isNotEmpty) {
+      final list = iterable.toList(growable: false);
+
+      if (list.isNotEmpty) {
         final index = _list.length;
-        _list.addAll(iterable);
-        _notifyRangeUpdate(index, iterable.toList(growable: false), null);
+
+        _list.addAll(list);
+        _notifyRangeUpdate(index, list, null);
       }
     }, _atom);
   }
@@ -222,9 +225,11 @@ class ObservableList<T>
   @override
   void insertAll(int index, Iterable<T> iterable) {
     _context.conditionallyRunInAction(() {
-      if (iterable.isNotEmpty) {
-        _list.insertAll(index, iterable);
-        _notifyRangeUpdate(index, iterable.toList(growable: false), null);
+      final list = iterable.toList(growable: false);
+
+      if (list.isNotEmpty) {
+        _list.insertAll(index, list);
+        _notifyRangeUpdate(index, list, null);
       }
     }, _atom);
   }
@@ -303,11 +308,13 @@ class ObservableList<T>
   @override
   void replaceRange(int start, int end, Iterable<T> newContents) {
     _context.conditionallyRunInAction(() {
-      if (end > start || newContents.isNotEmpty) {
+      final list = newContents.toList(growable: false);
+
+      if (end > start || list.isNotEmpty) {
         final oldContents = _list.sublist(start, end);
-        _list.replaceRange(start, end, newContents);
-        _notifyRangeUpdate(
-            start, newContents.toList(growable: false), oldContents);
+
+        _list.replaceRange(start, end, list);
+        _notifyRangeUpdate(start, list, oldContents);
       }
     }, _atom);
   }
@@ -333,10 +340,11 @@ class ObservableList<T>
   @override
   void setAll(int index, Iterable<T> iterable) {
     _context.conditionallyRunInAction(() {
-      if (iterable.isNotEmpty) {
-        final oldValues = _list.sublist(index, index + iterable.length);
-        final newValues = iterable.toList(growable: false);
-        _list.setAll(index, iterable);
+      final newValues = iterable.toList(growable: false);
+
+      if (newValues.isNotEmpty) {
+        final oldValues = _list.sublist(index, index + newValues.length);
+        _list.setAll(index, newValues);
         _notifyRangeUpdate(index, newValues, oldValues);
       }
     }, _atom);
@@ -347,9 +355,12 @@ class ObservableList<T>
     _context.conditionallyRunInAction(() {
       if (end > start) {
         final oldValues = _list.sublist(start, end);
+
         final newValues =
             iterable.skip(skipCount).take(end - start).toList(growable: false);
-        _list.setRange(start, end, iterable, skipCount);
+
+        _list.setRange(start, end, newValues);
+
         _notifyRangeUpdate(start, newValues, oldValues);
       }
     }, _atom);

diff --git a/mobx/lib/version.dart b/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.0';
+const version = '2.2.1';
diff --git a/mobx/pubspec.yaml b/mobx/pubspec.yaml
@@ -1,5 +1,5 @@
 name: mobx
-version: 2.2.0
+version: 2.2.1
 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

diff --git a/mobx/test/observable_list_test.dart b/mobx/test/observable_list_test.dart
@@ -811,6 +811,88 @@ void main() {
       'reversed': (_) => _.reversed
     }.forEach(_templateIterableReadTest);
   });
+
+  group('checks for iterable use with ObservableList', () {
+    test('ObservableList addAll runs Iterable only once', () {
+      final list = ObservableList<String>();
+      var op = '';
+
+      list.addAll([1, 2].map((e) {
+        op += '$e';
+
+        return '$e';
+      }));
+
+      expect(op, '12');
+      expect(list.join(''), '12');
+    });
+
+    test('ObservableList insertAll runs Iterable only once', () {
+      final list = ObservableList<String>.of(['1']);
+      var op = '';
+
+      list.insertAll(
+          1,
+          [2, 3].map((e) {
+            op += '$e';
+
+            return '$e';
+          }));
+
+      expect(op, '23');
+      expect(list.join(''), '123');
+    });
+
+    test('ObservableList replaceRange runs Iterable only once', () {
+      final list = ObservableList<String>.of(['1', '2', '3']);
+      var op = '';
+
+      list.replaceRange(
+          0,
+          2,
+          [4, 5].map((e) {
+            op += '$e';
+
+            return '$e';
+          }));
+
+      expect(op, '45');
+      expect(list.join(''), '453');
+    });
+
+    test('ObservableList setAll runs Iterable only once', () {
+      final list = ObservableList<String>.of(['1', '2', '3']);
+      var op = '';
+
+      list.setAll(
+          0,
+          [4, 5, 6].map((e) {
+            op += '$e';
+
+            return '$e';
+          }));
+
+      expect(op, '456');
+      expect(list.join(''), '456');
+    });
+
+    test('ObservableList setRange runs Iterable only once', () {
+      final list =
+          ObservableList<String>.of(['1', '2', '3', '4', '5', '6', '7']);
+      var op = '';
+
+      list.setRange(
+          1,
+          4,
+          [8, 9, 10].map((e) {
+            op += '$e';
+
+            return '$e';
+          }));
+
+      expect(op, '8910');
+    });
+  });
 }
 
 dynamic _ignoreException(Function fn) {