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 1/2] =?UTF-8?q?=F0=9F=90=9B=20Fixes=20issue=20in=20Observa?= =?UTF-8?q?bleList=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) { From 6641bf4d8e9bdcd7a273f336c60936dd3bf36790 Mon Sep 17 00:00:00 2001 From: "allcontributors[bot]" <46447321+allcontributors[bot]@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:52:06 +0530 Subject: [PATCH 2/2] add ParthBaraiya as a contributor for code, and bug (#945) * update README.md [skip ci] * update .all-contributorsrc [skip ci] --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> --- .all-contributorsrc | 13 ++++++++++++- README.md | 3 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index fcd92101c..9da76d534 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -598,8 +598,19 @@ "contributions": [ "doc" ] + }, + { + "login": "ParthBaraiya", + "name": "Parth Baraiya", + "avatar_url": "https://avatars.githubusercontent.com/u/36261739?v=4", + "profile": "https://github.com/ParthBaraiya", + "contributions": [ + "code", + "bug" + ] } ], "contributorsPerLine": 7, - "skipCi": true + "skipCi": true, + "commitType": "docs" } diff --git a/README.md b/README.md index da206c342..68cc62f82 100644 --- a/README.md +++ b/README.md @@ -405,7 +405,7 @@ the growing community of `MobX.dart`. ## Contributors ✨ -[![All Contributors](https://img.shields.io/badge/all_contributors-62-orange.svg?style=flat-square)](#contributors-) +[![All Contributors](https://img.shields.io/badge/all_contributors-63-orange.svg?style=flat-square)](#contributors-) Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/docs/en/emoji-key)): @@ -494,6 +494,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d Sergey Molchanovsky
Sergey Molchanovsky
💻 ko16a46
ko16a46
💻 Yatharth Chauhan
Yatharth Chauhan
📖 + Parth Baraiya
Parth Baraiya
💻 🐛