Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: observableset and observablemap notify all listeners when one is added with fireimmediately true #962

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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