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

Avoid unnecessary observable notifications of Iterable or Map fields #951

Merged
merged 5 commits into from
Dec 4, 2023
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.3

- Avoid unnecessary observable notifications of `@observable` `Iterable` or `Map` fields of Stores by [@amondnet](https://github.com/amondnet) in [#951](https://github.com/mobxjs/mobx.dart/pull/951)

## 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)
Expand Down
7 changes: 4 additions & 3 deletions mobx/lib/src/core/atom_extensions.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import 'package:mobx/mobx.dart';

import '../utils.dart';

extension AtomSpyReporter on Atom {
void reportRead() {
context.enforceReadPolicy(this);
Expand All @@ -8,11 +10,10 @@ extension AtomSpyReporter on Atom {

void reportWrite<T>(T newValue, T oldValue, void Function() setNewValue,
{EqualityComparer<T>? equals}) {
final areEqual =
equals == null ? oldValue == newValue : equals(oldValue, newValue);
final areEqual = equals ?? equatable;

// Avoid unnecessary observable notifications of @observable fields of Stores
if (areEqual) {
if (areEqual(newValue, oldValue)) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion mobx/lib/src/core/observable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Observable<T> extends Atom
}

final areEqual =
equals == null ? prepared == value : equals!(prepared, _value);
equals == null ? equatable(prepared, value) : equals!(prepared, _value);

return (!areEqual) ? prepared : WillChangeNotification.unchanged;
}
Expand Down
18 changes: 18 additions & 0 deletions mobx/lib/src/utils.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import 'dart:async';

import 'package:collection/collection.dart' show DeepCollectionEquality;

const Duration ms = Duration(milliseconds: 1);

Timer Function(void Function()) createDelayedScheduler(int delayMs) =>
Expand All @@ -20,3 +22,19 @@ mixin DebugCreationStack {
return result;
}();
}

/// Determines whether [a] and [b] are equal.
bool equatable<T>(T a, T b) {
if (identical(a, b)) return true;
if (a is Iterable || a is Map) {
if (!_equality.equals(a, b)) return false;
} else if (a.runtimeType != b.runtimeType) {
return false;
} else if (a != b) {
return false;
}

return true;
}

const DeepCollectionEquality _equality = DeepCollectionEquality();
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.2';
const version = '2.2.3';
4 changes: 2 additions & 2 deletions mobx/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: mobx
version: 2.2.2
version: 2.2.3
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 All @@ -10,10 +10,10 @@ environment:

dependencies:
meta: ^1.3.0
collection: ^1.15.0

dev_dependencies:
build_runner: ^2.0.6
collection: ^1.15.0
coverage: ^1.0.1
fake_async: ^1.2.0
lints: ^2.0.0
Expand Down
167 changes: 126 additions & 41 deletions mobx/test/atom_extensions_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,75 +4,123 @@ import 'package:test/test.dart';
void main() {
test(
'when write to @observable field with changed value, should trigger notifications for downstream',
() {
final store = _ExampleStore();
() {
final store = _ExampleStore();

final autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value));
final autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value));

expect(autorunResults, ['first']);
expect(autorunResults, ['first']);

store.value = 'second';
store.value = 'second';

expect(autorunResults, ['first', 'second']);
});
expect(autorunResults, ['first', 'second']);
});

// fixed by #855
test(
'when write to @observable field with unchanged value, should not trigger notifications for downstream',
() {
final store = _ExampleStore();
() {
final store = _ExampleStore();

final autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value));
final autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value));

expect(autorunResults, ['first']);
expect(autorunResults, ['first']);

store.value = store.value;
store.value = store.value;

expect(autorunResults, ['first']);
});
expect(autorunResults, ['first']);
});

test(
'when write to @alwaysNotify field with unchanged value, should trigger notifications for downstream',
() {
final store = _ExampleStore();
() {
final store = _ExampleStore();

final autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value2));
final autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value2));

expect(autorunResults, ['first']);
expect(autorunResults, ['first']);

store.value2 = store.value2;
store.value2 = store.value2;

expect(autorunResults, ['first', 'first']);
});
expect(autorunResults, ['first', 'first']);
});

test(
'when write to @MakeObservable(equals: "a?.length == b?.length") field with changed value and not equals, should trigger notifications for downstream',
() {
final store = _ExampleStore();
() {
final store = _ExampleStore();

final autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value3));
final autorunResults = <int>[];
autorun((_) => autorunResults.add(store.value3.length));

expect(autorunResults, ['first']); // length: 5
expect(autorunResults, [5]); // length: 5

// length: 5, should not trigger
store.value3 = 'third';
// length: 5, should not trigger
store.value3 = 'third';

expect(autorunResults, ['first']);
expect(autorunResults, [5]);

// length: 6, should trigger
store.value3 = 'second';
// length: 6, should trigger
store.value3 = 'second';

expect(autorunResults, ['first', 'second']);
});
expect(autorunResults, [5, 6]);
});

test(
'when write to iterable @observable field with unchanged value, should not trigger notifications for downstream',
() {
final store = _ExampleStore();

final autorunResults = <List<String>>[];
autorun((_) => autorunResults.add(store.list));

store.list = ['first'];
expect(autorunResults, [
['first']
]);

store.list = ['first'];
expect(autorunResults, [
['first']
]);

store.list = ['first'];
expect(autorunResults, [
['first']
]);
});

test(
'when write to map @observable field with unchanged value, should not trigger notifications for downstream',
() {
final store = _ExampleStore();

final autorunResults = <Map<String, int>>[];
autorun((_) => autorunResults.add(store.map));

store.map = {'first': 1};
expect(autorunResults, [
{'first': 1}
]);

store.map = {'first': 1};
expect(autorunResults, [
{'first': 1}
]);

store.map = {'first': 1};
expect(autorunResults, [
{'first': 1}
]);
});
}

class _ExampleStore = __ExampleStore with _$_ExampleStore;

bool _equals(String? oldValue, String? newValue) => (oldValue == newValue);
bool _equals(String? oldValue, String? newValue) => (oldValue?.length == newValue?.length);

abstract class __ExampleStore with Store {
@observable
Expand All @@ -83,6 +131,12 @@ abstract class __ExampleStore with Store {

@MakeObservable(equals: _equals)
String value3 = 'first';

@observable
List<String> list = ['first'];

@observable
Map<String, int> map = {'first': 1};
}

// This is what typically a mobx codegen will generate.
Expand All @@ -105,7 +159,7 @@ mixin _$_ExampleStore on __ExampleStore, Store {

// ignore: non_constant_identifier_names
late final _$value2Atom =
Atom(name: '__ExampleStore.value2', context: context);
Atom(name: '__ExampleStore.value2', context: context);

@override
String get value2 {
Expand All @@ -122,7 +176,7 @@ mixin _$_ExampleStore on __ExampleStore, Store {

// ignore: non_constant_identifier_names
late final _$value3Atom =
Atom(name: '__ExampleStore.value3', context: context);
Atom(name: '__ExampleStore.value3', context: context);

@override
String get value3 {
Expand All @@ -135,7 +189,38 @@ mixin _$_ExampleStore on __ExampleStore, Store {
_$value3Atom.reportWrite(value, super.value3, () {
super.value3 = value;
},
equals: (String? oldValue, String? newValue) =>
oldValue?.length == newValue?.length);
equals: _equals);
}

// ignore: non_constant_identifier_names
late final _$listAtom = Atom(name: '__ExampleStore.list', context: context);

@override
List<String> get list {
_$listAtom.reportRead();
return super.list;
}

@override
set list(List<String> value) {
_$listAtom.reportWrite(value, super.list, () {
super.list = value;
});
}

// ignore: non_constant_identifier_names
late final _$mapAtom = Atom(name: '__ExampleStore.map', context: context);

@override
Map<String, int> get map {
_$mapAtom.reportRead();
return super.map;
}

@override
set map(Map<String, int> value) {
_$mapAtom.reportWrite(value, super.map, () {
super.map = value;
});
}
}