From 6833ba1632fb9dca5700c7b98feeaf70c4b7e86d Mon Sep 17 00:00:00 2001 From: Parth Baraiya <36261739+ParthBaraiya@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:51:03 +0530 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixes=20issue=20in=20ObservableL?= =?UTF-8?q?ist=20while=20using=20Iterables.=20(#942)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- mobx/CHANGELOG.md | 4 + .../observable_list.dart | 41 ++++++---- mobx/lib/version.dart | 2 +- mobx/pubspec.yaml | 2 +- mobx/test/observable_list_test.dart | 82 +++++++++++++++++++ 5 files changed, 114 insertions(+), 17 deletions(-) diff --git a/mobx/CHANGELOG.md b/mobx/CHANGELOG.md index 5a2fcda84..52380b9dc 100644 --- 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 index 1175127c4..bca553fee 100644 --- 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 @override void addAll(Iterable 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 @override void insertAll(int index, Iterable 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 @override void replaceRange(int start, int end, Iterable 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 @override void setAll(int index, Iterable 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 _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 index 855f13a8b..ebfac1c17 100644 --- 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 index ae530a21e..bbf31ca52 100644 --- 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 index 52005e371..14fab84c9 100644 --- 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(); + 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.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.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.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.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) {