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

Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of @observable fields of Stores; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
880ee28
add `nonObservableInner`, such that users can bypass observability sy…
fzyzcjy Jul 12, 2022
5a58d06
Merge remote-tracking branch 'origin/master'
fzyzcjy Jul 12, 2022
e1df03a
chore: format code
fzyzcjy Sep 4, 2022
bbd9020
feat: Add `onDispose` callback to `Computed`, such that old values ca…
fzyzcjy Sep 4, 2022
263155b
test: add tests to `disposeValue` #857
fzyzcjy Sep 4, 2022
396cd93
test: add tests for "Allow users to bypass observability system for p…
fzyzcjy Sep 4, 2022
20fe5d3
feat: add `Computed.dispose` #859
fzyzcjy Sep 4, 2022
63d57ef
test: add tests for Computed.dispose
fzyzcjy Sep 4, 2022
ada24f7
test: reproduce #855
fzyzcjy Sep 5, 2022
3b93039
fix: Avoid unnecessary observable notifications of `@observable` fiel…
fzyzcjy Sep 5, 2022
919f203
Merge pull request #4 from mobxjs/master
fzyzcjy Sep 6, 2022
305bd4e
Revert "test: add tests for Computed.dispose"
fzyzcjy Sep 6, 2022
3912572
Revert "feat: add `Computed.dispose` #859"
fzyzcjy Sep 6, 2022
290f8c3
Revert "test: add tests to `disposeValue` #857"
fzyzcjy Sep 6, 2022
424ec02
Revert "feat: Add `onDispose` callback to `Computed`, such that old v…
fzyzcjy Sep 6, 2022
fcc222b
format code and minor change to code
fzyzcjy Sep 6, 2022
637d12b
add Observable.nonObservableValue
fzyzcjy Sep 7, 2022
e852b9b
fix: Reaction lacks toString, so cannot see which reaction causes the…
fzyzcjy Sep 8, 2022
75260bf
feat: Add StackTrace to reactions in debug mode to easily spot which …
fzyzcjy Sep 8, 2022
fecb442
fix linter errors
fzyzcjy Sep 8, 2022
1803928
add toString and debugCreationStack
fzyzcjy Sep 8, 2022
015d35f
Merge branch 'master' into master
amondnet Dec 19, 2022
74ea2e5
Merge branch 'master' into master
amondnet Jan 27, 2023
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
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 @@ -114,6 +116,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;
}();
}
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
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;
});
}
}
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
17 changes: 17 additions & 0 deletions mobx/test/observable_set_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:mobx/src/api/observable_collections.dart';
import 'package:mobx/src/api/reaction.dart';
import 'package:mobx/src/core.dart';
import 'package:mocktail/mocktail.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -143,6 +144,22 @@ void main() {
'retainWhere': (m) => m.retainWhere((i) => i < 3),
}.forEach(runWriteTest);
});

test('bypass observable system', () {
final set = ObservableSet<int>();

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

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

set.add(10);

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

Expand Down
21 changes: 21 additions & 0 deletions mobx/test/observable_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 @@ -12,6 +13,18 @@ void main() {
testSetup();

group('observable<T>', () {
test('toString', () {
final object = Observable(42, name: 'MyName');
expect(object.toString(), contains('MyName'));
});

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

test('basics work', () {
final x = Observable<int?>(null);
expect(x.value, equals(null));
Expand Down Expand Up @@ -59,6 +72,14 @@ void main() {
() => context.endBatch()
]);
});

test('nonObservableValue', () {
final x = Observable<int?>(null);
expect(x.nonObservableValue, null);

x.value = 100;
expect(x.nonObservableValue, 100);
});
});

group('createAtom()', () {
Expand Down
Loading