From f28fc632c6588ac4453d0a8eef8f219e8c056ba2 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 24 Nov 2023 19:43:13 +0900 Subject: [PATCH 1/3] fix: `ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately --- .../observable_map.dart | 12 ++++++--- .../observable_set.dart | 11 +++++--- mobx/test/observable_list_test.dart | 26 +++++++++++++++++++ mobx/test/observable_map_test.dart | 14 ++++++++++ mobx/test/observable_set_test.dart | 13 ++++++++++ 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/mobx/lib/src/api/observable_collections/observable_map.dart b/mobx/lib/src/api/observable_collections/observable_map.dart index be1cd7d4d..e5f31281b 100644 --- a/mobx/lib/src/api/observable_collections/observable_map.dart +++ b/mobx/lib/src/api/observable_collections/observable_map.dart @@ -219,11 +219,17 @@ class ObservableMap @override Dispose observe(MapChangeListener listener, {bool fireImmediately = false}) { - final dispose = _listeners.add(listener); if (fireImmediately == true) { - _map.forEach(_reportAdd); + _map.forEach((key, value) { + listener(MapChange( + type: OperationType.add, + key: key, + newValue: value, + object: this, + )); + }); } - return dispose; + return _listeners.add(listener); } } diff --git a/mobx/lib/src/api/observable_collections/observable_set.dart b/mobx/lib/src/api/observable_collections/observable_set.dart index 65cc56e86..409445459 100644 --- a/mobx/lib/src/api/observable_collections/observable_set.dart +++ b/mobx/lib/src/api/observable_collections/observable_set.dart @@ -153,11 +153,16 @@ class ObservableSet @override Dispose observe(SetChangeListener listener, {bool fireImmediately = false}) { - final dispose = _listeners.add(listener); if (fireImmediately == true) { - _set.forEach(_reportAdd); + for (final value in _set) { + listener(SetChange( + object: this, + type: OperationType.add, + value: value, + )); + } } - return dispose; + return _listeners.add(listener); } void _reportAdd(T value) { diff --git a/mobx/test/observable_list_test.dart b/mobx/test/observable_list_test.dart index 14fab84c9..90553e539 100644 --- a/mobx/test/observable_list_test.dart +++ b/mobx/test/observable_list_test.dart @@ -103,6 +103,32 @@ void main() { expect(count, equals(2)); }); + test( + 'observe with fireImmediately should not send events to already registered listeners', + () { + final list = ObservableList.of([0]); + + var count1 = 0; + var count2 = 0; + + list + .observe((change) { + count1++; + }); + + list + .observe((change) { + count2++; + }, fireImmediately: true); + + list.add(1); + + // 0 + 1: add + expect(count1, equals(1)); + // 1 + 1: fireImmediately + add + expect(count2, equals(2)); + }); + test('observe set length works', () { // ignore: omit_local_variable_types final ObservableList list = ObservableList.of([0]); diff --git a/mobx/test/observable_map_test.dart b/mobx/test/observable_map_test.dart index b057d83e8..71a99559e 100644 --- a/mobx/test/observable_map_test.dart +++ b/mobx/test/observable_map_test.dart @@ -222,6 +222,20 @@ void main() { expect(changes[1].newValue, equals(1)); }); + test( + 'observe should not send changes to all listeners immediately when fireImmediately is true', + () { + final changes1 = []; + final changes2 = []; + final map = ObservableMap.of({'a': 0, 'b': 1}); + + map.observe(changes1.add); + map.observe(changes2.add, fireImmediately: true); + + expect(changes1, isEmpty); + expect(changes2, isNotEmpty); + }); + test('works when adding a null value', () { final map = ObservableMap(); map['a'] = null; diff --git a/mobx/test/observable_set_test.dart b/mobx/test/observable_set_test.dart index cf4180bef..c5778463c 100644 --- a/mobx/test/observable_set_test.dart +++ b/mobx/test/observable_set_test.dart @@ -86,6 +86,19 @@ void main() { expect(changes[1].value, equals(2)); }); + test( + 'observe should not send add events to all listeners when fireImmediately is true', + () { + final oset = ObservableSet.of([1, 2]); + final changes1 = >[]; + final changes2 = >[]; + oset.observe(changes1.add); + oset.observe(changes2.add, fireImmediately: true); + + expect(changes1, isEmpty); + expect(changes2, isNotEmpty); + }); + group('fires reportObserved() for read methods', () { )>{ 'union': (m) => m.union({2, 5, 6}), From 1cf0ed2fef1f09e2a3558908d27eb814da3429e8 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 24 Nov 2023 19:45:32 +0900 Subject: [PATCH 2/3] fix: `ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately --- mobx/CHANGELOG.md | 4 ++++ mobx/lib/version.dart | 2 +- mobx/pubspec.yaml | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/mobx/CHANGELOG.md b/mobx/CHANGELOG.md index 52380b9dc..6196cd678 100644 --- a/mobx/CHANGELOG.md +++ b/mobx/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.2.2 + +- Fix #956: ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately. by [@amondnet](https://github.com/amondnet) + ## 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). diff --git a/mobx/lib/version.dart b/mobx/lib/version.dart index ebfac1c17..ff46c3cb7 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.1'; +const version = '2.2.2'; diff --git a/mobx/pubspec.yaml b/mobx/pubspec.yaml index bbf31ca52..e8abae7bc 100644 --- a/mobx/pubspec.yaml +++ b/mobx/pubspec.yaml @@ -1,5 +1,5 @@ name: mobx -version: 2.2.1 +version: 2.2.2 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 From 46a7c26e9cd58b39cc7c4fced96f03629b27b7fd Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 24 Nov 2023 19:47:03 +0900 Subject: [PATCH 3/3] fix: `ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately --- mobx/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mobx/CHANGELOG.md b/mobx/CHANGELOG.md index 6196cd678..3eabc66a7 100644 --- a/mobx/CHANGELOG.md +++ b/mobx/CHANGELOG.md @@ -1,6 +1,6 @@ ## 2.2.2 -- Fix #956: ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately. by [@amondnet](https://github.com/amondnet) +- 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) ## 2.2.1