Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
* add `nonObservableInner`, such that users can bypass observability system for performance

* chore: format code

* feat: Add `onDispose` callback to `Computed`, such that old values can be properly disposed #857

* test: add tests to `disposeValue` #857

* test: add tests for "Allow users to bypass observability system for performance"

* feat: add `Computed.dispose` #859

* test: add tests for Computed.dispose

* test: reproduce #855

* fix: Avoid unnecessary observable notifications of `@observable` fields of `Store`s #855

* Revert "test: add tests for Computed.dispose"

This reverts commit 63d57ef.

* Revert "feat: add `Computed.dispose` #859"

This reverts commit 20fe5d3.

* Revert "test: add tests to `disposeValue` #857"

This reverts commit 263155b.

# Conflicts:
#	mobx/test/computed_test.dart

* Revert "feat: Add `onDispose` callback to `Computed`, such that old values can be properly disposed #857"

This reverts commit bbd9020.

* format code and minor change to code

* add Observable.nonObservableValue

* fix: Reaction lacks toString, so cannot see which reaction causes the error #864

* feat: Add StackTrace to reactions in debug mode to easily spot which reaction it is #864

* fix linter errors

* add toString and debugCreationStack

* test: add atom_test

---------

Co-authored-by: fzyzcjy <ch271828n@outlook.com>
Co-authored-by: fzyzcjy <5236035+fzyzcjy@users.noreply.github.com>
  • Loading branch information
3 people authored Feb 16, 2023
1 parent 558449a commit d99a5c6
Show file tree
Hide file tree
Showing 24 changed files with 273 additions and 6 deletions.
7 changes: 7 additions & 0 deletions mobx/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 2.1.4

- Allow users to bypass observability system for performance by [@fzyzcjy](https://github.com/fzyzcjy) in [#844](https://github.com/mobxjs/mobx.dart/pull/844)
- Avoid unnecessary observable notifications of @observable fields of Stores by [@fzyzcjy](https://github.com/fzyzcjy) in [#844](https://github.com/mobxjs/mobx.dart/pull/844)
- Fix Reaction lacks toString, so cannot see which reaction causes the error by [@fzyzcjy](https://github.com/fzyzcjy) in [#844](https://github.com/mobxjs/mobx.dart/pull/844)
- Add StackTrace to reactions in debug mode to easily spot which reaction it is by [@fzyzcjy](https://github.com/fzyzcjy) in [#844](https://github.com/mobxjs/mobx.dart/pull/844)

## 2.1.2 - 2.1.3+1

- Fix tests in dart 2.19 - [@amondnet](https://github.com/amondnet)
Expand Down
2 changes: 2 additions & 0 deletions mobx/lib/src/api/observable_collections/observable_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class ObservableList<T>
final Atom _atom;
final List<T> _list;

List<T> get nonObservableInner => _list;

Listeners<ListChange<T>>? _listenersField;

Listeners<ListChange<T>> get _listeners =>
Expand Down
2 changes: 2 additions & 0 deletions mobx/lib/src/api/observable_collections/observable_map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class ObservableMap<K, V>
final Atom _atom;
final Map<K, V> _map;

Map<K, V> get nonObservableInner => _map;

String get name => _atom.name;

Listeners<MapChange<K, V>>? _listenersField;
Expand Down
2 changes: 2 additions & 0 deletions mobx/lib/src/api/observable_collections/observable_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class ObservableSet<T>
final Atom _atom;
final Set<T> _set;

Set<T> get nonObservableInner => _set;

String get name => _atom.name;

Listeners<SetChange<T>>? _listenersField;
Expand Down
6 changes: 5 additions & 1 deletion mobx/lib/src/core/action.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
part of '../core.dart';

class Action {
class Action with DebugCreationStack {
/// Creates an action that encapsulates all the mutations happening on the
/// observables.
///
Expand Down Expand Up @@ -61,6 +61,10 @@ class Action {
_controller.endAction(runInfo);
}
}

@override
String toString() =>
'Action(name: $name, identity: ${identityHashCode(this)})';
}

/// `ActionController` is used to define the start/end boundaries of code which
Expand Down
7 changes: 6 additions & 1 deletion mobx/lib/src/core/atom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ enum _ListenerKind {
onBecomeUnobserved,
}

class Atom {
class Atom with DebugCreationStack {
/// Creates a simple Atom for tracking its usage in a reactive context. This is useful when
/// you don't need the value but instead a way of knowing when it becomes active and inactive
/// in a reaction.
Expand Down Expand Up @@ -42,6 +42,8 @@ class Atom {

DerivationState _lowestObserverState = DerivationState.notTracking;

bool get isBeingObserved => _isBeingObserved;

// ignore: prefer_final_fields
bool _isBeingObserved = false;

Expand Down Expand Up @@ -110,6 +112,9 @@ class Atom {
}
};
}

@override
String toString() => 'Atom(name: $name, identity: ${identityHashCode(this)})';
}

class WillChangeNotification<T> {
Expand Down
5 changes: 5 additions & 0 deletions mobx/lib/src/core/atom_extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ extension AtomSpyReporter on Atom {
}

void reportWrite<T>(T newValue, T oldValue, void Function() setNewValue) {
// Avoid unnecessary observable notifications of @observable fields of Stores
if (newValue == oldValue) {
return;
}

context.spyReport(ObservableValueSpyEvent(this,
newValue: newValue, oldValue: oldValue, name: name));

Expand Down
4 changes: 4 additions & 0 deletions mobx/lib/src/core/computed.dart
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,8 @@ class Computed<T> extends Atom implements Derivation, ObservableValue<T> {
}, context: _context)
.call;
}

@override
String toString() =>
'Computed<$T>(name: $name, identity: ${identityHashCode(this)})';
}
4 changes: 3 additions & 1 deletion mobx/lib/src/core/context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,9 @@ class ReactiveContext {
_resetState();

throw MobXCyclicReactionException(
"Reaction doesn't converge to a stable state after ${config.maxIterations} iterations. Probably there is a cycle in the reactive function: $failingReaction");
"Reaction doesn't converge to a stable state after ${config.maxIterations} iterations. "
"Probably there is a cycle in the reactive function: $failingReaction "
"(creation stack: ${failingReaction.debugCreationStack})");
}

final remainingReactions = allReactions.toList(growable: false);
Expand Down
6 changes: 6 additions & 0 deletions mobx/lib/src/core/observable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class Observable<T> extends Atom
return _value;
}

T get nonObservableValue => _value;

set value(T value) {
_context.enforceWritePolicy(this);

Expand Down Expand Up @@ -125,4 +127,8 @@ class Observable<T> extends Atom
@override
Dispose intercept(Interceptor<T> interceptor) =>
_interceptors.add(interceptor);

@override
String toString() =>
'Observable<$T>(name: $name, identity: ${identityHashCode(this)})';
}
8 changes: 7 additions & 1 deletion mobx/lib/src/core/reaction.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ abstract class Reaction implements Derivation {
void dispose();

void _run();

StackTrace? get debugCreationStack;
}

class ReactionImpl implements Reaction {
class ReactionImpl with DebugCreationStack implements Reaction {
ReactionImpl(this._context, Function() onInvalidate,
{required this.name, void Function(Object, Reaction)? onError}) {
_onInvalidate = onInvalidate;
Expand Down Expand Up @@ -180,4 +182,8 @@ class ReactionImpl implements Reaction {

_context._notifyReactionErrorHandlers(exception, this);
}

@override
String toString() =>
'Reaction(name: $name, identity: ${identityHashCode(this)})';
}
16 changes: 16 additions & 0 deletions mobx/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,19 @@ const Duration ms = Duration(milliseconds: 1);

Timer Function(void Function()) createDelayedScheduler(int delayMs) =>
(fn) => Timer(ms * delayMs, fn);

mixin DebugCreationStack {
/// Set the flag to true, to enable [debugCreationStack].
/// Otherwise, the stack is always null.
static var enable = false;

/// The stack trace when the object is created
final StackTrace? debugCreationStack = () {
StackTrace? result;
assert(() {
if (enable) result = StackTrace.current;
return true;
}());
return result;
}();
}
3 changes: 2 additions & 1 deletion mobx/lib/version.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Generated via set_version.dart. !!!DO NOT MODIFY BY HAND!!!

/// The current version as per `pubspec.yaml`.
const version = '2.1.3+1';
const version = '2.1.4';
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.1.3+1
version: 2.1.4
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
13 changes: 13 additions & 0 deletions mobx/test/action_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:mobx/mobx.dart';
import 'package:mobx/src/utils.dart';
import 'package:mocktail/mocktail.dart' as mock;
import 'package:test/test.dart';

Expand All @@ -11,6 +12,18 @@ void main() {
testSetup();

group('Action', () {
test('toString', () {
final object = Action(() {}, name: 'MyName');
expect(object.toString(), contains('MyName'));
});

test('debugCreationStack', () {
DebugCreationStack.enable = true;
addTearDown(() => DebugCreationStack.enable = false);
final object = Action(() {});
expect(object.debugCreationStack, isNotNull);
});

test('basics work', () {
final a = Action((String name, String value) {
expect(name, equals('name'));
Expand Down
3 changes: 3 additions & 0 deletions mobx/test/all_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import 'reactive_policies_test.dart' as reactive_policies_test;
import 'spy_test.dart' as spy_test;
import 'store_test.dart' as store_test;
import 'when_test.dart' as when_test;
import 'atom_test.dart' as atom_test;

void main() {
observable_test.main();
Expand Down Expand Up @@ -71,4 +72,6 @@ void main() {

spy_test.main();
store_test.main();

atom_test.main();
}
61 changes: 61 additions & 0 deletions mobx/test/atom_extensions_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import 'package:mobx/mobx.dart';
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 autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value));

expect(autorunResults, ['first']);

store.value = '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 autorunResults = <String>[];
autorun((_) => autorunResults.add(store.value));

expect(autorunResults, ['first']);

store.value = store.value;

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

class _ExampleStore = __ExampleStore with _$_ExampleStore;

abstract class __ExampleStore with Store {
@observable
String value = 'first';
}

// This is what typically a mobx codegen will generate.
mixin _$_ExampleStore on __ExampleStore, Store {
// ignore: non_constant_identifier_names
late final _$valueAtom = Atom(name: '__ExampleStore.value', context: context);

@override
String get value {
_$valueAtom.reportRead();
return super.value;
}

@override
set value(String value) {
_$valueAtom.reportWrite(value, super.value, () {
super.value = value;
});
}
}
31 changes: 31 additions & 0 deletions mobx/test/atom_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import 'package:mobx/mobx.dart';
import 'package:mobx/src/utils.dart';
import 'package:test/test.dart';

import 'util.dart';

void main() {
testSetup();

group('Atom', () {
test('toString', () {
final object = Atom(name: 'MyName');
expect(object.toString(), contains('MyName'));
});

test('debugCreationStack', () {
DebugCreationStack.enable = true;
addTearDown(() => DebugCreationStack.enable = false);
final object = Atom();
expect(object.debugCreationStack, isNotNull);
});

test('isBeingObserved', () {
final observable = Observable(1, name: 'MyName');
expect(observable.isBeingObserved, false);
final d = autorun((_) => observable.value);
expect(observable.isBeingObserved, true);
d();
});
});
}
13 changes: 13 additions & 0 deletions mobx/test/computed_test.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:collection/collection.dart';
import 'package:mobx/mobx.dart' hide when;
import 'package:mobx/src/utils.dart';
import 'package:mocktail/mocktail.dart';
import 'package:test/test.dart';

Expand All @@ -10,6 +11,18 @@ void main() {
testSetup();

group('Computed', () {
test('toString', () {
final object = Computed(() {}, name: 'MyName');
expect(object.toString(), contains('MyName'));
});

test('debugCreationStack', () {
DebugCreationStack.enable = true;
addTearDown(() => DebugCreationStack.enable = false);
final object = Computed(() {});
expect(object.debugCreationStack, isNotNull);
});

test('basics work', () {
final x = Observable(20);
final y = Observable(10);
Expand Down
17 changes: 17 additions & 0 deletions mobx/test/observable_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,23 @@ void main() {
'[]': (_) => _[0],
'+': (_) => _ + [100],
}.forEach(_templateReadTest);

test('bypass observable system', () {
final list = ObservableList<int>();

int? nonObservableInnerLength;
autorun(
(_) => nonObservableInnerLength = list.nonObservableInner.length);

expect(list.nonObservableInner.length, 0);
expect(nonObservableInnerLength, equals(0));

list.add(20);

expect(list.nonObservableInner.length, 1);
expect(nonObservableInnerLength, equals(0),
reason: 'should not be observable');
});
});
});

Expand Down
16 changes: 16 additions & 0 deletions mobx/test/observable_map_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,22 @@ void main() {

expect(map.containsKey('a'), isTrue);
});

test('bypass observable system', () {
final map = ObservableMap<int, int>();

int? nonObservableInnerLength;
autorun((_) => nonObservableInnerLength = map.nonObservableInner.length);

expect(map.nonObservableInner.length, 0);
expect(nonObservableInnerLength, equals(0));

map[10] = 20;

expect(map.nonObservableInner.length, 1);
expect(nonObservableInnerLength, equals(0),
reason: 'should not be observable');
});
});
}

Expand Down
Loading

0 comments on commit d99a5c6

Please sign in to comment.