Skip to content

Commit

Permalink
fix: observableset and observablemap notify all listeners when one is…
Browse files Browse the repository at this point in the history
… added with fireimmediately true (#962)

* fix: `ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately

* fix: `ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately

* fix: `ObservableSet` and `ObservableMap` should not notify all listeners when `observe` with fireImmediately

---------

Co-authored-by: Pavan Podila <[email protected]>
  • Loading branch information
amondnet and pavanpodila authored Nov 30, 2023
1 parent 74e4a89 commit a00d4da
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 8 deletions.
4 changes: 4 additions & 0 deletions mobx/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 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)

## 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).
Expand Down
12 changes: 9 additions & 3 deletions mobx/lib/src/api/observable_collections/observable_map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,17 @@ class ObservableMap<K, V>
@override
Dispose observe(MapChangeListener<K, V> listener,
{bool fireImmediately = false}) {
final dispose = _listeners.add(listener);
if (fireImmediately == true) {
_map.forEach(_reportAdd);
_map.forEach((key, value) {
listener(MapChange<K, V>(
type: OperationType.add,
key: key,
newValue: value,
object: this,
));
});
}
return dispose;
return _listeners.add(listener);
}
}

Expand Down
11 changes: 8 additions & 3 deletions mobx/lib/src/api/observable_collections/observable_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,16 @@ class ObservableSet<T>
@override
Dispose observe(SetChangeListener<T> 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) {
Expand Down
2 changes: 1 addition & 1 deletion mobx/lib/version.dart
Original file line number Diff line number Diff line change
@@ -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';
2 changes: 1 addition & 1 deletion mobx/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
26 changes: 26 additions & 0 deletions mobx/test/observable_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<int?> list = ObservableList.of([0]);
Expand Down
14 changes: 14 additions & 0 deletions mobx/test/observable_map_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <MapChange>[];
final changes2 = <MapChange>[];
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;
Expand Down
13 changes: 13 additions & 0 deletions mobx/test/observable_set_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <SetChange<int>>[];
final changes2 = <SetChange<int>>[];
oset.observe(changes1.add);
oset.observe(changes2.add, fireImmediately: true);

expect(changes1, isEmpty);
expect(changes2, isNotEmpty);
});

group('fires reportObserved() for read methods', () {
<String, void Function(ObservableSet<int>)>{
'union': (m) => m.union({2, 5, 6}),
Expand Down

0 comments on commit a00d4da

Please sign in to comment.