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

Tracking changes between start and end #19

Merged
merged 3 commits into from
Jan 7, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 15 additions & 0 deletions lib/src/core/context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,21 @@ class ReactiveContext {
}
}

Derivation startTracking(Derivation derivation) {
final prevDerivation = _state.trackingDerivation;
_state.trackingDerivation = derivation;

resetDerivationState(derivation);
derivation.newObservables = Set();

return prevDerivation;
}

void endTracking(Derivation currentDerivation, Derivation prevDerivation) {
_state.trackingDerivation = prevDerivation;
bindDependencies(currentDerivation);
}

T trackDerivation<T>(Derivation d, T Function() fn) {
final prevDerivation = _state.trackingDerivation;
_state.trackingDerivation = d;
Expand Down
23 changes: 23 additions & 0 deletions lib/src/core/reaction.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,29 @@ import 'package:mobx/src/core/atom.dart';
import 'package:mobx/src/core/context.dart';
import 'package:mobx/src/core/derivation.dart';

class TrackingReaction extends Reaction {
TrackingReaction(ReactiveContext context, Function() onInvalidate,
{String name})
: super(context, onInvalidate, name: name);

Derivation startTracking() {
_context.startBatch();
_isRunning = true;
return _context.startTracking(this);
}

void endTracking(Derivation previous) {
_context.endTracking(this, previous);
_isRunning = false;

if (_isDisposed) {
_context.clearObservables(this);
}

_context.endBatch();
}
}

class Reaction implements Derivation {
Reaction(this._context, Function() onInvalidate, {this.name}) {
_onInvalidate = onInvalidate;
Expand Down
40 changes: 40 additions & 0 deletions lib/src/core/tracking.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import 'package:meta/meta.dart';
import 'package:mobx/src/core/context.dart';
import 'package:mobx/src/core/derivation.dart';
import 'package:mobx/src/core/reaction.dart';

/// Tracks changes that happen between [Tracking.begin] and [Tracking.end].
///
/// This should only be used in situations where it is not possible to
/// track changes inside a callback function.
@experimental
class Tracking {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a more explicit name like DerivationTracker ? Going for a track-er rather than track-ing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use it in the current Reaction, ComputedValue implementations ? I think this would be an internally useful extraction of the tracking behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll refactor context a bit, add most of the logic there, but I think this should be the public (and unsafe) API of doing tracking without callbacks. The callback API should be used for 99.9% use cases, even in Reaction & ComputedValue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree about using callbacks as they are more natural. Internally we can have a callback interface to the start-end blocks of tracking. That way we are dog-fooding this Tracker class and it becomes a core behavior (which it is really).

I would think of this as the antithesis of untracked(). So a callback would be wrapped in tracked(), which internally uses the Tracker with the start/end blocks.

Here is a sample implementation of tracked (not tested).

dynamic tracked(void Function() fn, {ReactiveContext ctx}) {
  final tracker = Tracker(context: ctx);
  Tracker.start(); // ... 
  final result = fn();
  Tracker.end(); // ...
  return result;
}

Tracking.begin(ReactiveContext context, Function() onInvalidate,
{String name})
: assert(context != null),
assert(onInvalidate != null),
assert(name != ''),
_reaction = TrackingReaction(context, onInvalidate,
name: name ?? context.nameFor('Transaction')) {
_prevDerivation = _reaction.startTracking();
}

final TrackingReaction _reaction;
Derivation _prevDerivation;
bool _ended = false;

void end() {
if (_ended) {
return;
}

_ended = true;
_reaction.endTracking(_prevDerivation);
_prevDerivation = null;
}

void dispose() {
end();
_reaction.dispose();
}
}
1 change: 1 addition & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ homepage: https://github.com/mobxjs/mobx.dart

environment:
sdk: '>=2.1.0-dev <3.0.0'
meta: ^1.1.7

dev_dependencies:
test: ^1.5.1
Expand Down
2 changes: 2 additions & 0 deletions test/all.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'listenable_test.dart' as listenable_test;
import 'observable_test.dart' as observable_test;
import 'observe_test.dart' as observe_test;
import 'reaction_test.dart' as reaction_test;
import 'tracking.dart' as tracking_test;
import 'when_test.dart' as when_test;

void main() {
Expand All @@ -18,4 +19,5 @@ void main() {
observe_test.main();
intercept_test.main();
listenable_test.main();
tracking_test.main();
}
57 changes: 57 additions & 0 deletions test/tracking.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import 'package:mobx/mobx.dart';
import 'package:mobx/src/api/context.dart';
import 'package:mobx/src/core/tracking.dart';
import 'package:test/test.dart';

void main() {
test('Tracking reacts to changes to reactive values between begin and end',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also have a test with a autorun() to ensure the reactions are kicked off when the observables change outside

() {
final var1 = observable(0);
final var2 = observable(0);
var i = 0;

final tracking = Tracking.begin(currentContext, () {
i++;
});
final var3 = observable(0);
var1.value;
tracking.end();

// No changes, no calls to onInvalidate
expect(i, equals(0));

// Change outside tracking, no onInvalidate call
var2.value += 1;
expect(i, equals(0));

// Change outside tracking to an observable created inside tracking
// no onInvalidate call
var3.value += 1;
expect(i, equals(0));

// Changing a value that was read when tracking was active
// calls onInvalidate
var1.value += 1;
expect(i, equals(1));

// No calls to onInvalidate after first change
var1.value += 1;
expect(i, equals(1));
});

test("disposed Tracking doesn't call onInvalidate", () {
final var1 = observable(0);

var i = 0;
final tracking = Tracking.begin(currentContext, () {
i++;
});
var1.value;
tracking
..end()
..dispose();

var1.value += 1;
expect(i, equals(0));
});
}