Skip to content

Commit

Permalink
- Fix bug in auto dispose where signal causes cycle
Browse files Browse the repository at this point in the history
- Fix bug in auto dispose where signal causes cycle
- add private reset
- dispose does not reset to initial value and will read the last value with a warning
  • Loading branch information
rodydavis committed Feb 6, 2024
1 parent bb64dcb commit b38e884
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 15 deletions.
5 changes: 3 additions & 2 deletions packages/signals_core/lib/src/async/signal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ class AsyncSignal<T> extends ValueSignal<AsyncState<T>> {
_completer = Completer<bool>();
}

void reset() {
value = _initialValue;
@override
void reset([AsyncState<T>? value]) {
this.value = value ?? _initialValue;
_initialized = false;
_completer = Completer<bool>();
}
Expand Down
4 changes: 2 additions & 2 deletions packages/signals_core/lib/src/async/stream.dart
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ class StreamSignal<T> extends AsyncSignal<T> {
}

@override
void reset() {
super.reset();
void reset([AsyncState<T>? value]) {
super.reset(value);
_fetching = false;
_done = false;
_subscription?.cancel();
Expand Down
15 changes: 11 additions & 4 deletions packages/signals_core/lib/src/core/computed.dart
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ class _Computed<T> implements Computed<T>, _Listenable {
}
}
}
if (autoDispose && _allTargets.isEmpty) {
dispose();
}
}

@override
Expand Down Expand Up @@ -239,7 +242,8 @@ class _Computed<T> implements Computed<T>, _Listenable {
T get value {
if (disposed) {
if (kDebugMode) {
print('computed warning: [$globalId|$debugLabel] has been read after disposed');
print(
'computed warning: [$globalId|$debugLabel] has been read after disposed');
}
return this._value;
}
Expand Down Expand Up @@ -296,19 +300,22 @@ class _Computed<T> implements Computed<T>, _Listenable {

@override
void dispose() {
if (disposed) return;
for (final cleanup in _disposeCallbacks) {
cleanup();
}
_disposeCallbacks.clear();
_flags |= DISPOSED;
if (_node != null) _unsubscribe(_node!);
_value = _initialValue;
_previousValue = _initialValue;
disposed = true;
}

@override
T get initialValue => _initialValue;

void reset([T? value]) {
_value = value ?? _initialValue;
_previousValue = value ?? _initialValue;
}
}

typedef ComputedCallback<T> = T Function();
Expand Down
19 changes: 12 additions & 7 deletions packages/signals_core/lib/src/core/signal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,15 @@ class _Signal<T> extends Signal<T> {
signal._targets = next;
}
}
if (signal.autoDispose && signal._allTargets.isEmpty) {
signal.dispose();
}
}

@override
void _unsubscribe(_Node node) => __unsubscribe(this, node);
void _unsubscribe(_Node node) {
__unsubscribe(this, node);
if (autoDispose && _allTargets.isEmpty) {
dispose();
}
}

@override
EffectCleanup subscribe(void Function(T value) fn) {
Expand Down Expand Up @@ -363,14 +365,17 @@ class _Signal<T> extends Signal<T> {

@override
void dispose() {
if (disposed) return;
for (final cleanup in _disposeCallbacks) {
cleanup();
}
if (_node != null) _unsubscribe(_node!);
_value = _initialValue;
_previousValue = _initialValue;
disposed = true;
}

void reset([T? value]) {
_value = value ?? _initialValue;
_previousValue = value ?? _initialValue;
}
}

/// The `signal` function creates a new signal. A signal is a container
Expand Down
39 changes: 39 additions & 0 deletions packages/signals_core/test/core/signals_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,45 @@ void main() {
expect(() => s.value = [3], throwsA(isA<SignalsError>()));
// expect(() => s.value, throwsA(isA<SignalsError>()));
});

test('should autoDispose signal when it has no listeners', () {
var count = signal(2, autoDispose: true);
var multiple = signal(2);
var product = computed(() => count() * multiple());

expect(count.value, 2);
expect(multiple.value, 2);
expect(product.value, 4);

count.onDispose(() => print('disposed count'));
product.onDispose(() => print('disposed doubled'));

count.value = 3;
multiple.value = 3;

expect(count.value, 3);
expect(multiple.value, 3);
expect(product.value, 9);

// simulate widget listeners
var unmount = effect(() {
print('count: ${count.value}');
print('product: ${product.value}');
});

// simulate widget unmount
unmount();

expect(count.disposed, isTrue);
expect(multiple.disposed, isFalse);
expect(product.disposed, isFalse);

multiple.value = 4; // update signal that's not autodisposed

print('count: ${product.value}');

expect(product.value, 12);
});
});

test('should inherit from Signal', () {
Expand Down

0 comments on commit b38e884

Please sign in to comment.