From 3a9fc98e6ec9861a442d6161a6d63fb9e5d36208 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 15 Dec 2023 18:51:09 +0900 Subject: [PATCH 1/3] Allow a custom equals parameter for observable collections --- mobx/CHANGELOG.md | 4 + mobx/lib/src/api/observable_collections.dart | 2 + .../observable_list.dart | 43 +++++--- .../observable_map.dart | 49 +++++++--- .../observable_set.dart | 98 ++++++++++++++----- mobx/lib/version.dart | 2 +- mobx/pubspec.yaml | 2 +- 7 files changed, 147 insertions(+), 53 deletions(-) diff --git a/mobx/CHANGELOG.md b/mobx/CHANGELOG.md index ba7a7a9e7..613d1eedf 100644 --- a/mobx/CHANGELOG.md +++ b/mobx/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.6.0 + +- Allow a custom equals parameter for observable collections( ObservableList, ObservableMap, ObservableSet ) - [@amondnet](https://github.com/amondnet) + ## 2.5.0 - **FIX**: package upgrades, analysis issue fixes. diff --git a/mobx/lib/src/api/observable_collections.dart b/mobx/lib/src/api/observable_collections.dart index 6a6361634..f4e9b4cfc 100644 --- a/mobx/lib/src/api/observable_collections.dart +++ b/mobx/lib/src/api/observable_collections.dart @@ -1,8 +1,10 @@ import 'dart:collection'; import 'dart:math'; +import 'package:collection/collection.dart'; import 'package:meta/meta.dart'; import 'package:mobx/mobx.dart'; +import 'package:mobx/src/utils.dart'; part 'observable_collections/observable_list.dart'; part 'observable_collections/observable_map.dart'; diff --git a/mobx/lib/src/api/observable_collections/observable_list.dart b/mobx/lib/src/api/observable_collections/observable_list.dart index bca553fee..c93de7dee 100644 --- a/mobx/lib/src/api/observable_collections/observable_list.dart +++ b/mobx/lib/src/api/observable_collections/observable_list.dart @@ -25,20 +25,23 @@ class ObservableList ListMixin implements Listenable> { - ObservableList({ReactiveContext? context, String? name}) - : this._wrap(context, _observableListAtom(context, name), []); + ObservableList( + {ReactiveContext? context, String? name, EqualityComparer? equals}) + : this._wrap(context, _observableListAtom(context, name), [], equals); ObservableList.of(Iterable elements, - {ReactiveContext? context, String? name}) + {ReactiveContext? context, String? name, EqualityComparer? equals}) : this._wrap(context, _observableListAtom(context, name), - List.of(elements, growable: true)); + List.of(elements, growable: true), equals); - ObservableList._wrap(ReactiveContext? context, this._atom, this._list) + ObservableList._wrap( + ReactiveContext? context, this._atom, this._list, this._equals) : _context = context ?? mainContext; final ReactiveContext _context; final Atom _atom; final List _list; + final EqualityComparer? _equals; List get nonObservableInner => _list; @@ -96,7 +99,7 @@ class ObservableList _context.conditionallyRunInAction(() { final oldValue = _list[index]; - if (oldValue != value) { + if (!_areEquals(oldValue, value)) { _list[index] = value; _notifyElementUpdate(index, value, oldValue); } @@ -167,10 +170,18 @@ class ObservableList } @override - Map asMap() => ObservableMap._wrap(_context, _list.asMap(), _atom); + Map asMap() => + ObservableMap._wrap(_context, _list.asMap(), _atom, _equals); @override - List cast() => ObservableList._wrap(_context, _atom, _list.cast()); + List cast([EqualityComparer? equals]) => ObservableList._wrap( + _context, + _atom, + _list.cast(), + equals ?? + (_equals != null + ? (R? a, R? b) => _equals!(a as T?, b as T?) + : null)); @override List toList({bool growable = true}) { @@ -184,7 +195,7 @@ class ObservableList set first(T value) { _context.conditionallyRunInAction(() { final oldValue = _list.first; - if (oldValue != value) { + if (!_areEquals(oldValue, value)) { _list.first = value; _notifyElementUpdate(0, value, oldValue); } @@ -376,7 +387,7 @@ class ObservableList for (var i = 0; i < _list.length; ++i) { final oldValue = oldList[i]; final newValue = _list[i]; - if (newValue != oldValue) { + if (!_areEquals(oldValue, newValue)) { changes.add(ElementChange( index: i, oldValue: oldValue, newValue: newValue)); } @@ -398,7 +409,7 @@ class ObservableList for (var i = 0; i < _list.length; ++i) { final oldValue = oldList[i]; final newValue = _list[i]; - if (newValue != oldValue) { + if (!_areEquals(oldValue, newValue)) { changes.add(ElementChange( index: i, oldValue: oldValue, newValue: newValue)); } @@ -456,6 +467,14 @@ class ObservableList _listeners.notifyListeners(change); } + + bool _areEquals(T? a, T? b) { + if (_equals != null) { + return _equals!(a, b); + } else { + return equatable(a, b); + } + } } typedef ListChangeListener = void Function( @@ -520,4 +539,4 @@ class ListChange { /// Used during testing for wrapping a regular `List` as an `ObservableList` @visibleForTesting ObservableList wrapInObservableList(Atom atom, List list) => - ObservableList._wrap(mainContext, atom, list); + ObservableList._wrap(mainContext, atom, list, null); diff --git a/mobx/lib/src/api/observable_collections/observable_map.dart b/mobx/lib/src/api/observable_collections/observable_map.dart index e5f31281b..785d33fc3 100644 --- a/mobx/lib/src/api/observable_collections/observable_map.dart +++ b/mobx/lib/src/api/observable_collections/observable_map.dart @@ -26,37 +26,45 @@ class ObservableMap MapMixin implements Listenable> { - ObservableMap({ReactiveContext? context, String? name}) + ObservableMap( + {ReactiveContext? context, String? name, EqualityComparer? equals}) : _context = context ?? mainContext, _atom = _observableMapAtom(context, name), - _map = {}; + _map = {}, + _equals = equals; - ObservableMap.of(Map other, {ReactiveContext? context, String? name}) + ObservableMap.of(Map other, + {ReactiveContext? context, String? name, EqualityComparer? equals}) : _context = context ?? mainContext, _atom = _observableMapAtom(context, name), - _map = Map.of(other); + _map = Map.of(other), + _equals = equals; ObservableMap.linkedHashMapFrom(Map other, - {ReactiveContext? context, String? name}) + {ReactiveContext? context, String? name, EqualityComparer? equals}) : _context = context ?? mainContext, _atom = _observableMapAtom(context, name), - _map = LinkedHashMap.from(other); + _map = LinkedHashMap.from(other), + _equals = equals; ObservableMap.splayTreeMapFrom(Map other, {int Function(K, K)? compare, // ignore: avoid_annotating_with_dynamic bool Function(dynamic)? isValidKey, ReactiveContext? context, - String? name}) + String? name, + EqualityComparer? equals}) : _context = context ?? mainContext, _atom = _observableMapAtom(context, name), - _map = SplayTreeMap.from(other, compare, isValidKey); + _map = SplayTreeMap.from(other, compare, isValidKey), + _equals = equals; - ObservableMap._wrap(this._context, this._map, this._atom); + ObservableMap._wrap(this._context, this._map, this._atom, this._equals); final ReactiveContext _context; final Atom _atom; final Map _map; + final EqualityComparer? _equals; Map get nonObservableInner => _map; @@ -94,7 +102,7 @@ class ObservableMap } } - if (!_map.containsKey(key) || value != oldValue) { + if (!_map.containsKey(key) || !_areEquals(value, oldValue)) { _map[key] = value; if (type == 'update') { _reportUpdate(key, value, oldValue); @@ -127,8 +135,15 @@ class ObservableMap Iterable get keys => MapKeysIterable(_map.keys, _atom); @override - Map cast() => - ObservableMap._wrap(_context, super.cast(), _atom); + Map cast([EqualityComparer? equals]) => + ObservableMap._wrap( + _context, + super.cast(), + _atom, + equals ?? + (_equals == null + ? null + : (RV? a, RV? b) => _equals!(a as V?, b as V?))); @override V? remove(Object? key) { @@ -231,13 +246,21 @@ class ObservableMap } return _listeners.add(listener); } + + bool _areEquals(V? a, V? b) { + if (_equals != null) { + return _equals!(a, b); + } else { + return equatable(a, b); + } + } } /// A convenience method to wrap the standard `Map` in an `ObservableMap`. /// This is mostly to aid in testing. @visibleForTesting ObservableMap wrapInObservableMap(Atom atom, Map map) => - ObservableMap._wrap(mainContext, map, atom); + ObservableMap._wrap(mainContext, map, atom, null); typedef MapChangeListener = void Function(MapChange); diff --git a/mobx/lib/src/api/observable_collections/observable_set.dart b/mobx/lib/src/api/observable_collections/observable_set.dart index 409445459..1b404b72a 100644 --- a/mobx/lib/src/api/observable_collections/observable_set.dart +++ b/mobx/lib/src/api/observable_collections/observable_set.dart @@ -21,30 +21,36 @@ class ObservableSet SetMixin implements Listenable> { - ObservableSet({ReactiveContext? context, String? name}) - : this._(context ?? mainContext, {}, name); + ObservableSet( + {ReactiveContext? context, String? name, EqualityComparer? equals}) + : this._(context ?? mainContext, {}, name, equals); - ObservableSet.of(Iterable other, {ReactiveContext? context, String? name}) - : this._(context ?? mainContext, Set.of(other), name); + ObservableSet.of(Iterable other, + {ReactiveContext? context, String? name, EqualityComparer? equals}) + : this._(context ?? mainContext, Set.of(other), name, equals); ObservableSet.splayTreeSetFrom(Iterable other, {int Function(T, T)? compare, // ignore:avoid_annotating_with_dynamic bool Function(dynamic)? isValidKey, ReactiveContext? context, - String? name}) + String? name, + EqualityComparer? equals}) : this._(context ?? mainContext, - SplayTreeSet.of(other, compare, isValidKey), name); + SplayTreeSet.of(other, compare, isValidKey), name, equals); - ObservableSet._wrap(this._context, this._atom, this._set); + ObservableSet._wrap(this._context, this._atom, this._set, this._equals); - ObservableSet._(this._context, Set wrapped, String? name) + ObservableSet._( + this._context, Set wrapped, String? name, EqualityComparer? equals) : _atom = _observableSetAtom(_context, name), - _set = wrapped; + _set = wrapped, + _equals = equals; final ReactiveContext _context; final Atom _atom; final Set _set; + final EqualityComparer? _equals; Set get nonObservableInner => _set; @@ -63,26 +69,47 @@ class ObservableSet var result = false; _context.conditionallyRunInAction(() { - result = _set.add(value); + final oldValue = _equals != null ? _findValue(value) : null; - if (result && _hasListeners) { - _reportAdd(value); - } - - if (result) { - _atom.reportChanged(); + if (oldValue == null) { + result = _tryAddValue(value); } }, _atom); return result; } + T? _findValue(Object? value) { + if (value is T?) { + return _set.firstWhereOrNull((e) => _areEquals(e, value)); + } else { + return null; + } + } + + bool _tryAddValue(T value) { + var result = _set.add(value); + if (result) { + _reportChanges(value); + } + return result; + } + + void _reportChanges(T value) { + if (_hasListeners) { + _reportAdd(value); + } + _atom.reportChanged(); + } + @override bool contains(Object? element) { _context.enforceReadPolicy(_atom); _atom.reportObserved(); - return _set.contains(element); + return _equals == null + ? _set.contains(element) + : _findValue(element) != null; } @override @@ -101,7 +128,7 @@ class ObservableSet _context.enforceReadPolicy(_atom); _atom.reportObserved(); - return _set.lookup(element); + return _equals == null ? _set.lookup(element) : _findValue(element); } @override @@ -109,14 +136,18 @@ class ObservableSet var removed = false; _context.conditionallyRunInAction(() { - removed = _set.remove(value); + final oldValue = _equals != null ? _findValue(value) : value; - if (removed && _hasListeners) { - _reportRemove(value as T?); - } + if (oldValue != null) { + removed = _set.remove(oldValue); - if (removed) { - _atom.reportChanged(); + if (removed && _hasListeners) { + _reportRemove(oldValue as T?); + } + + if (removed) { + _atom.reportChanged(); + } } }, _atom); @@ -138,7 +169,14 @@ class ObservableSet } @override - Set cast() => ObservableSet._wrap(_context, _atom, _set.cast()); + Set cast([EqualityComparer? equals]) => ObservableSet._wrap( + _context, + _atom, + _set.cast(), + equals ?? + (_equals == null + ? null + : (R? a, R? b) => _equals!(a as T?, b as T?))); @override Set toSet() { @@ -180,13 +218,21 @@ class ObservableSet value: value, )); } + + bool _areEquals(T? a, T? b) { + if (_equals != null) { + return _equals!(a, b); + } else { + return equatable(a, b); + } + } } /// A convenience method used during unit testing. It creates an [ObservableSet] with a custom instance /// of an [Atom] @visibleForTesting ObservableSet wrapInObservableSet(Atom atom, Set set) => - ObservableSet._wrap(mainContext, atom, set); + ObservableSet._wrap(mainContext, atom, set, null); /// An internal iterator used to ensure that every read is tracked as part of the /// MobX reactivity system. diff --git a/mobx/lib/version.dart b/mobx/lib/version.dart index bc245f3fc..26dc6bd5b 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.5.0'; +const version = '2.6.0'; diff --git a/mobx/pubspec.yaml b/mobx/pubspec.yaml index 7b70b0632..1d14c39d1 100644 --- a/mobx/pubspec.yaml +++ b/mobx/pubspec.yaml @@ -1,5 +1,5 @@ name: mobx -version: 2.5.0 +version: 2.6.0 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." repository: https://github.com/mobxjs/mobx.dart From fda521756709f86cb87e96d873f3206d2992478c Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Sun, 28 Sep 2025 18:20:32 +0000 Subject: [PATCH 2/3] test: add unit tests for custom equals functionality in observable collections --- .../observable_list.dart | 28 + .../observable_map.dart | 18 + .../observable_set.dart | 21 + .../observable_collections_equals_test.dart | 551 ++++++++++++++++++ 4 files changed, 618 insertions(+) create mode 100644 mobx/test/observable_collections_equals_test.dart diff --git a/mobx/lib/src/api/observable_collections/observable_list.dart b/mobx/lib/src/api/observable_collections/observable_list.dart index c93de7dee..9e3affa14 100644 --- a/mobx/lib/src/api/observable_collections/observable_list.dart +++ b/mobx/lib/src/api/observable_collections/observable_list.dart @@ -10,6 +10,34 @@ Atom _observableListAtom(ReactiveContext? context, String? name) { /// /// As the name suggests, this is the Observable-counterpart to the standard Dart `List`. /// +/// ## Custom Equality +/// +/// You can provide a custom `equals` parameter to control when values are considered +/// equal. This is useful for optimizing change detection or implementing custom +/// equality semantics: +/// +/// ```dart +/// // Only notify changes when names are different +/// final list = ObservableList( +/// equals: (a, b) => a?.name == b?.name +/// ); +/// +/// // Deep equality for nested structures +/// final list = ObservableList>( +/// equals: (a, b) { +/// if (a == null && b == null) return true; +/// if (a == null || b == null) return false; +/// return a.length == b.length && +/// a.asMap().entries.every((e) => e.value == b[e.key]); +/// } +/// ); +/// ``` +/// +/// Note: Bulk operations like `replaceRange` and `setRange` always trigger +/// notifications for performance reasons, regardless of the custom equals function. +/// +/// ## Basic Usage +/// /// ```dart /// final list = ObservableList.of([1]); /// diff --git a/mobx/lib/src/api/observable_collections/observable_map.dart b/mobx/lib/src/api/observable_collections/observable_map.dart index 785d33fc3..e7702be06 100644 --- a/mobx/lib/src/api/observable_collections/observable_map.dart +++ b/mobx/lib/src/api/observable_collections/observable_map.dart @@ -10,6 +10,24 @@ Atom _observableMapAtom(ReactiveContext? context, String? name) { /// /// As the name suggests, this is the Observable-counterpart to the standard Dart `Map`. /// +/// ## Custom Equality for Values +/// +/// You can provide a custom `equals` parameter to control when values are considered +/// equal. This only affects value comparisons - keys are always compared using standard +/// equality: +/// +/// ```dart +/// // Only notify changes when person names are different +/// final map = ObservableMap( +/// equals: (a, b) => a?.name == b?.name +/// ); +/// +/// map['key'] = Person('Alice', 25); +/// map['key'] = Person('Alice', 30); // No notification - same name +/// ``` +/// +/// ## Basic Usage +/// /// ```dart /// final map = ObservableMap.of({'first': 1}); /// diff --git a/mobx/lib/src/api/observable_collections/observable_set.dart b/mobx/lib/src/api/observable_collections/observable_set.dart index 1b404b72a..8f96c7927 100644 --- a/mobx/lib/src/api/observable_collections/observable_set.dart +++ b/mobx/lib/src/api/observable_collections/observable_set.dart @@ -5,6 +5,27 @@ Atom _observableSetAtom(ReactiveContext context, String? name) => /// ObservableSet provides a reactive set that notifies changes when a member is added or removed. /// +/// ## Custom Equality +/// +/// You can provide a custom `equals` parameter to determine set membership. +/// This is particularly useful when you need custom equality semantics: +/// +/// ```dart +/// // Case-insensitive string set +/// final set = ObservableSet( +/// equals: (a, b) => a?.toLowerCase() == b?.toLowerCase() +/// ); +/// +/// set.add('Hello'); +/// set.add('HELLO'); // Won't be added - considered equal +/// set.contains('hello'); // Returns true +/// ``` +/// +/// When using custom equals, the set behaves consistently across all operations +/// including `add`, `contains`, `remove`, and `lookup`. +/// +/// ## Basic Usage +/// /// ```dart /// final set = ObservableSet.of([1, 2, 3]); /// diff --git a/mobx/test/observable_collections_equals_test.dart b/mobx/test/observable_collections_equals_test.dart new file mode 100644 index 000000000..699246a31 --- /dev/null +++ b/mobx/test/observable_collections_equals_test.dart @@ -0,0 +1,551 @@ +import 'package:mobx/mobx.dart'; +import 'package:test/test.dart'; + +import 'util.dart'; + +// Custom class for testing equals +class Person { + final String name; + final int age; + + Person(this.name, this.age); + + @override + String toString() => 'Person($name, $age)'; +} + +void main() { + testSetup(); + + group('ObservableList with custom equals', () { + test('uses custom equals for value comparisons', () { + // Custom equals that only compares names + bool nameEquals(Person? a, Person? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.name == b.name; + } + + final list = ObservableList(equals: nameEquals); + var changeCount = 0; + + final dispose = autorun((_) { + // Trigger observation + list.toList(); + changeCount++; + }); + + list.add(Person('Alice', 25)); + expect(changeCount, equals(2)); // Initial + add + + // Update with same name but different age - should not trigger change + list[0] = Person('Alice', 30); + expect(changeCount, equals(2)); // No change due to custom equals + + // Update with different name - should trigger change + list[0] = Person('Bob', 30); + expect(changeCount, equals(3)); // Changed + + dispose(); + }); + + test('custom equals works with null values', () { + bool customEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.toLowerCase() == b.toLowerCase(); + } + + final list = ObservableList(equals: customEquals); + list.add(null); + list.add('Hello'); + + var changeCount = 0; + final dispose = autorun((_) { + list.toList(); + changeCount++; + }); + + // Same value (null) + list[0] = null; + expect(changeCount, equals(1)); // No change + + // Different case but equal according to custom equals + list[1] = 'HELLO'; + expect(changeCount, equals(1)); // No change + + // Actually different value + list[1] = 'World'; + expect(changeCount, equals(2)); // Changed + + dispose(); + }); + + test('cast preserves custom equals', () { + bool numEquals(num? a, num? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.round() == b.round(); + } + + final list = ObservableList(equals: numEquals); + list.add(1.4); + list.add(2.6); + + final intList = list.cast((int? a, int? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a == b; + }); + + var changeCount = 0; + final dispose = autorun((_) { + list.toList(); + changeCount++; + }); + + // Update with value that rounds to same + list[0] = 1.3; + expect(changeCount, equals(1)); // No change due to rounding equals + + list[0] = 2.1; + expect(changeCount, equals(2)); // Changed (rounds to 2, not 1) + + dispose(); + }); + + test('first setter uses custom equals', () { + bool caseInsensitiveEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.toLowerCase() == b.toLowerCase(); + } + + final list = ObservableList.of(['hello', 'world'], + equals: caseInsensitiveEquals); + + var changeCount = 0; + final dispose = autorun((_) { + list.first; + changeCount++; + }); + + list.first = 'HELLO'; // Same value with different case + expect(changeCount, equals(1)); // No change + + list.first = 'goodbye'; + expect(changeCount, equals(2)); // Changed + + dispose(); + }); + + test('replaceRange always triggers notifications (bulk operation)', () { + // Note: replaceRange is a bulk operation that always triggers + // notifications, even if individual values are equal according to + // the custom equals function. This is by design for performance. + bool lengthEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.length == b.length; + } + + final list = ObservableList.of(['aa', 'bb', 'cc'], + equals: lengthEquals); + + var changeCount = 0; + final dispose = autorun((_) { + list.toList(); + changeCount++; + }); + + // Replace with same length strings - still triggers notification + list.replaceRange(0, 2, ['xx', 'yy']); + expect(changeCount, equals(2)); // Bulk operation always notifies + + // Replace with different length strings + list.replaceRange(0, 1, ['xxx']); + expect(changeCount, equals(3)); // Changed + + dispose(); + }); + + test('setRange always triggers notifications (bulk operation)', () { + // Note: setRange is a bulk operation that always triggers + // notifications, even if values are equal according to custom equals + bool modEquals(int? a, int? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a % 10 == b % 10; + } + + final list = ObservableList.of([1, 2, 3, 4], equals: modEquals); + + var changeCount = 0; + final dispose = autorun((_) { + list.toList(); + changeCount++; + }); + + // Set range with values that have same mod 10 - still triggers + list.setRange(0, 2, [11, 12]); + expect(changeCount, equals(2)); // Bulk operation always notifies + + // Set range with different mod values + list.setRange(0, 2, [5, 6]); + expect(changeCount, equals(3)); // Changed + + dispose(); + }); + }); + + group('ObservableMap with custom equals', () { + test('uses custom equals for value comparisons', () { + bool personNameEquals(Person? a, Person? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.name == b.name; + } + + final map = ObservableMap(equals: personNameEquals); + var changeCount = 0; + + final dispose = autorun((_) { + map.values.toList(); + changeCount++; + }); + + map['key1'] = Person('Alice', 25); + expect(changeCount, equals(2)); // Initial + add + + // Update with same name but different age + map['key1'] = Person('Alice', 30); + expect(changeCount, equals(2)); // No change due to custom equals + + // Update with different name + map['key1'] = Person('Bob', 30); + expect(changeCount, equals(3)); // Changed + + dispose(); + }); + + test('custom equals works with null values in map', () { + bool customEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.toLowerCase() == b.toLowerCase(); + } + + final map = ObservableMap(equals: customEquals); + map['key1'] = null; + map['key2'] = 'Hello'; + + var changeCount = 0; + final dispose = autorun((_) { + map.values.toList(); + changeCount++; + }); + + // Same value (null) + map['key1'] = null; + expect(changeCount, equals(1)); // No change + + // Different case but equal + map['key2'] = 'HELLO'; + expect(changeCount, equals(1)); // No change + + // Actually different value + map['key2'] = 'World'; + expect(changeCount, equals(2)); // Changed + + dispose(); + }); + + test('cast preserves custom equals in map', () { + bool numEquals(num? a, num? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.round() == b.round(); + } + + final map = ObservableMap(equals: numEquals); + map['a'] = 1.4; + map['b'] = 2.6; + + final intMap = map.cast((int? a, int? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a == b; + }); + + var changeCount = 0; + final dispose = autorun((_) { + map.values.toList(); + changeCount++; + }); + + // Update with value that rounds to same + map['a'] = 1.3; + expect(changeCount, equals(1)); // No change + + map['a'] = 2.1; + expect(changeCount, equals(2)); // Changed + + dispose(); + }); + + test('addAll uses custom equals', () { + bool lengthEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.length == b.length; + } + + final map = ObservableMap(equals: lengthEquals); + map['a'] = 'xx'; + map['b'] = 'yy'; + + var changeCount = 0; + final dispose = autorun((_) { + map.values.toList(); + changeCount++; + }); + + // Add values with same length - should not trigger change for existing + map.addAll({'a': 'aa', 'c': 'zz'}); + expect(changeCount, equals(2)); // Only new key 'c' triggers change + expect(map['a'], equals('xx')); // Original value preserved + + dispose(); + }); + }); + + group('ObservableSet with custom equals', () { + test('uses custom equals for add operations', () { + bool caseInsensitiveEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.toLowerCase() == b.toLowerCase(); + } + + final set = ObservableSet(equals: caseInsensitiveEquals); + var changeCount = 0; + + final dispose = autorun((_) { + set.toList(); + changeCount++; + }); + + set.add('hello'); + expect(changeCount, equals(2)); // Initial + add + expect(set.length, equals(1)); + + // Try to add same value with different case + final added = set.add('HELLO'); + expect(added, isFalse); // Not added due to custom equals + expect(changeCount, equals(2)); // No change + expect(set.length, equals(1)); + + set.add('world'); + expect(changeCount, equals(3)); // Changed + expect(set.length, equals(2)); + + dispose(); + }); + + test('contains uses custom equals', () { + bool personNameEquals(Person? a, Person? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.name == b.name; + } + + final set = ObservableSet(equals: personNameEquals); + final alice1 = Person('Alice', 25); + final alice2 = Person('Alice', 30); + final bob = Person('Bob', 25); + + set.add(alice1); + + // Contains should use custom equals + expect(set.contains(alice2), isTrue); // Same name, different age + expect(set.contains(bob), isFalse); + }); + + test('remove uses custom equals', () { + bool lengthEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.length == b.length; + } + + final set = ObservableSet(equals: lengthEquals); + set.add('hello'); + set.add('ab'); + + var changeCount = 0; + final dispose = autorun((_) { + set.toList(); + changeCount++; + }); + + // Remove using different string with same length + final removed = set.remove('world'); // Same length as 'hello' + expect(removed, isTrue); + expect(changeCount, equals(2)); // Changed + expect(set.length, equals(1)); + expect(set.contains('ab'), isTrue); + + dispose(); + }); + + test('lookup uses custom equals', () { + bool caseInsensitiveEquals(String? a, String? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.toLowerCase() == b.toLowerCase(); + } + + final set = ObservableSet(equals: caseInsensitiveEquals); + set.add('Hello'); + set.add('World'); + + // Lookup should return the actual stored value + expect(set.lookup('HELLO'), equals('Hello')); + expect(set.lookup('world'), equals('World')); + expect(set.lookup('notfound'), isNull); + }); + + test('cast preserves custom equals in set', () { + bool numEquals(num? a, num? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a.round() == b.round(); + } + + final set = ObservableSet(equals: numEquals); + set.add(1.4); + set.add(2.6); + + final intSet = set.cast((int? a, int? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a == b; + }); + + expect(set.length, equals(2)); + + // Try to add value that rounds to existing + final added = set.add(1.3); + expect(added, isFalse); // Not added due to rounding equals + expect(set.length, equals(2)); + }); + + test('addAll uses custom equals', () { + bool modEquals(int? a, int? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + return a % 10 == b % 10; + } + + final set = ObservableSet(equals: modEquals); + set.add(1); + set.add(2); + + var changeCount = 0; + final dispose = autorun((_) { + set.toList(); + changeCount++; + }); + + // Add values with same mod 10 + set.addAll([11, 22, 3]); // 11 same as 1, 22 same as 2, 3 is new + expect(changeCount, equals(2)); // Only value 3 triggers change + expect(set.length, equals(3)); + + dispose(); + }); + }); + + group('Edge cases and performance', () { + test('equals function is called efficiently', () { + var equalsCallCount = 0; + bool countingEquals(int? a, int? b) { + equalsCallCount++; + return a == b; + } + + final list = ObservableList(equals: countingEquals); + list.addAll([1, 2, 3]); + equalsCallCount = 0; // Reset counter + + // Setting same value should call equals once + list[0] = 1; + expect(equalsCallCount, equals(1)); + + // Setting different value should call equals once + list[0] = 10; + expect(equalsCallCount, equals(2)); + }); + + test('default behavior when equals is null', () { + final list1 = ObservableList(); + final list2 = ObservableList(equals: null); + + list1.add('hello'); + list2.add('hello'); + + var changes1 = 0; + var changes2 = 0; + + final d1 = autorun((_) { + list1.toList(); + changes1++; + }); + + final d2 = autorun((_) { + list2.toList(); + changes2++; + }); + + list1[0] = 'hello'; // Same value + list2[0] = 'hello'; // Same value + + expect(changes1, equals(1)); // No change (default equality) + expect(changes2, equals(1)); // No change (null equals uses default) + + d1(); + d2(); + }); + + test('equals with complex nested structures', () { + bool deepEquals(List? a, List? b) { + if (a == null && b == null) return true; + if (a == null || b == null) return false; + if (a.length != b.length) return false; + for (var i = 0; i < a.length; i++) { + if (a[i] != b[i]) return false; + } + return true; + } + + final list = ObservableList>(equals: deepEquals); + list.add([1, 2, 3]); + + var changeCount = 0; + final dispose = autorun((_) { + list.toList(); + changeCount++; + }); + + // Update with equal list (different instance) + list[0] = [1, 2, 3]; + expect(changeCount, equals(1)); // No change + + // Update with different list + list[0] = [1, 2, 4]; + expect(changeCount, equals(2)); // Changed + + dispose(); + }); + }); +} \ No newline at end of file From c6b6515258926d9c2a4a5fe23d8ea78243bfb9a5 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Sun, 28 Sep 2025 18:25:27 +0000 Subject: [PATCH 3/3] refactor: simplify casting logic in equals tests --- mobx/test/observable_collections_equals_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mobx/test/observable_collections_equals_test.dart b/mobx/test/observable_collections_equals_test.dart index 699246a31..f8fa84277 100644 --- a/mobx/test/observable_collections_equals_test.dart +++ b/mobx/test/observable_collections_equals_test.dart @@ -92,7 +92,7 @@ void main() { list.add(1.4); list.add(2.6); - final intList = list.cast((int? a, int? b) { + list.cast((int? a, int? b) { if (a == null && b == null) return true; if (a == null || b == null) return false; return a == b; @@ -271,7 +271,7 @@ void main() { map['a'] = 1.4; map['b'] = 2.6; - final intMap = map.cast((int? a, int? b) { + map.cast((int? a, int? b) { if (a == null && b == null) return true; if (a == null || b == null) return false; return a == b; @@ -426,7 +426,7 @@ void main() { set.add(1.4); set.add(2.6); - final intSet = set.cast((int? a, int? b) { + set.cast((int? a, int? b) { if (a == null && b == null) return true; if (a == null || b == null) return false; return a == b;