diff --git a/packages/riverpod/CHANGELOG.md b/packages/riverpod/CHANGELOG.md index d5fe7418c..5ded22860 100644 --- a/packages/riverpod/CHANGELOG.md +++ b/packages/riverpod/CHANGELOG.md @@ -24,6 +24,8 @@ - **Breaking**: A provider is now considered "paused" if all of its listeners are also paused. So if a provider `A` is watched _only_ by a provider `B`, and `B` is currently unused, then `A` will be paused. +- All `Ref` life-cycles (such as `Ref.onDispose`) and `Notifier.listenSelf` + now return a function to remove the listener. - Added methods to `ProviderObserver` for listening to "mutations". Mutations are a new code-generation-only feature. See riverpod_generator's changelog for more information. diff --git a/packages/riverpod/lib/riverpod.dart b/packages/riverpod/lib/riverpod.dart index 439a701d0..531661a21 100644 --- a/packages/riverpod/lib/riverpod.dart +++ b/packages/riverpod/lib/riverpod.dart @@ -22,7 +22,6 @@ export 'src/framework.dart' ProviderElementProxy, OnError, ProviderContainerTest, - DebugRiverpodDevtoolBiding, TransitiveFamilyOverride, TransitiveProviderOverride, $ProviderPointer, diff --git a/packages/riverpod/lib/src/core/devtool.dart b/packages/riverpod/lib/src/core/devtool.dart deleted file mode 100644 index 6e483608d..000000000 --- a/packages/riverpod/lib/src/core/devtool.dart +++ /dev/null @@ -1,17 +0,0 @@ -part of '../framework.dart'; - -@internal -abstract class DebugRiverpodDevtoolBiding { - static final _containers = []; - - static List get containers => - UnmodifiableListView(_containers); - - static void addContainer(ProviderContainer container) { - _containers.add(container); - } - - static void removeContainer(ProviderContainer container) { - _containers.remove(container); - } -} diff --git a/packages/riverpod/lib/src/core/provider/notifier_provider.dart b/packages/riverpod/lib/src/core/provider/notifier_provider.dart index c671fcdea..b387f128d 100644 --- a/packages/riverpod/lib/src/core/provider/notifier_provider.dart +++ b/packages/riverpod/lib/src/core/provider/notifier_provider.dart @@ -65,12 +65,14 @@ abstract class NotifierBase { /// As opposed to [Ref.listen], the listener will be called even if /// [updateShouldNotify] returns false, meaning that the previous /// and new value can potentially be identical. + /// + /// Returns a function which can be called to remove the listener. @protected - void listenSelf( + RemoveListener listenSelf( void Function(StateT? previous, StateT next) listener, { void Function(Object error, StackTrace stackTrace)? onError, }) { - $ref.listenSelf(listener, onError: onError); + return $ref.listenSelf(listener, onError: onError); } @visibleForTesting diff --git a/packages/riverpod/lib/src/core/provider_container.dart b/packages/riverpod/lib/src/core/provider_container.dart index 4954a782e..9900be895 100644 --- a/packages/riverpod/lib/src/core/provider_container.dart +++ b/packages/riverpod/lib/src/core/provider_container.dart @@ -636,7 +636,6 @@ class ProviderContainer implements Node { // This ensures that if an error is thrown, the parent & global state // are not affected. parent?._children.add(this); - if (kDebugMode) DebugRiverpodDevtoolBiding.addContainer(this); } /// An automatically disposed [ProviderContainer]. @@ -954,10 +953,6 @@ class ProviderContainer implements Node { for (final element in getAllProviderElementsInOrder().toList().reversed) { element.dispose(); } - - if (kDebugMode) { - DebugRiverpodDevtoolBiding.removeContainer(this); - } } /// Release all the resources associated with this [ProviderContainer]. diff --git a/packages/riverpod/lib/src/core/ref.dart b/packages/riverpod/lib/src/core/ref.dart index d48e4a879..b36fcf019 100644 --- a/packages/riverpod/lib/src/core/ref.dart +++ b/packages/riverpod/lib/src/core/ref.dart @@ -257,24 +257,32 @@ final = Provider(dependencies: []); /// A life-cycle for whenever a new listener is added to the provider. /// + /// Returns a function which can be called to remove the listener. + /// /// See also: /// - [onRemoveListener], for when a listener is removed - void onAddListener(void Function() cb) { + RemoveListener onAddListener(void Function() cb) { _throwIfInvalidUsage(); - _onAddListeners ??= []; - _onAddListeners!.add(cb); + final list = _onAddListeners ??= []; + list.add(cb); + + return () => list.remove(cb); } /// A life-cycle for whenever a listener is removed from the provider. /// + /// Returns a function which can be called to remove the listener. + /// /// See also: /// - [onAddListener], for when a listener is added - void onRemoveListener(void Function() cb) { + RemoveListener onRemoveListener(void Function() cb) { _throwIfInvalidUsage(); - _onRemoveListeners ??= []; - _onRemoveListeners!.add(cb); + final list = _onRemoveListeners ??= []; + list.add(cb); + + return () => list.remove(cb); } /// Add a listener to perform an operation when the last listener of the provider @@ -287,6 +295,8 @@ final = Provider(dependencies: []); /// _will_ get paused/dispose. It is possible that after the last listener /// is removed, a new listener is immediately added. /// + /// Returns a function which can be called to remove the listener. + /// /// See also: /// - [keepAlive], which can be combined with [onCancel] for /// advanced manipulation on when the provider should get disposed. @@ -294,16 +304,20 @@ final = Provider(dependencies: []); /// destroy its state when no longer listened to. /// - [onDispose], a life-cycle for when a provider is disposed. /// - [onResume], a life-cycle for when the provider is listened to again. - void onCancel(void Function() cb) { + RemoveListener onCancel(void Function() cb) { _throwIfInvalidUsage(); - _onCancelListeners ??= []; - _onCancelListeners!.add(cb); + final list = _onCancelListeners ??= []; + list.add(cb); + + return () => list.remove(cb); } /// A life-cycle for when a provider is listened again after it was paused /// (and [onCancel] was triggered). /// + /// Returns a function which can be called to remove the listener. + /// /// See also: /// - [keepAlive], which can be combined with [onCancel] for /// advanced manipulation on when the provider should get disposed. @@ -311,11 +325,13 @@ final = Provider(dependencies: []); /// destroy its state when no longer listened to. /// - [onDispose], a life-cycle for when a provider is disposed. /// - [onCancel], a life-cycle for when all listeners of a provider are removed. - void onResume(void Function() cb) { + RemoveListener onResume(void Function() cb) { _throwIfInvalidUsage(); - _onResumeListeners ??= []; - _onResumeListeners!.add(cb); + final list = _onResumeListeners ??= []; + list.add(cb); + + return () => list.remove(cb); } /// Adds a listener to perform an operation right before the provider is destroyed. @@ -357,6 +373,8 @@ final = Provider(dependencies: []); /// if an exception happens before [onDispose] is called, then /// some of your objects may not be disposed. /// + /// Returns a function which can be called to remove the listener. + /// /// See also: /// /// - [Provider.autoDispose], a modifier which tell a provider that it should @@ -364,11 +382,13 @@ final = Provider(dependencies: []); /// - [ProviderContainer.dispose], to destroy all providers associated with /// a [ProviderContainer] at once. /// - [onCancel], a life-cycle for when all listeners of a provider are removed. - void onDispose(void Function() listener) { + RemoveListener onDispose(void Function() listener) { _throwIfInvalidUsage(); - _onDisposeListeners ??= []; - _onDisposeListeners!.add(listener); + final list = _onDisposeListeners ??= []; + list.add(listener); + + return () => list.remove(listener); } /// Read the state associated with a provider, without listening to that provider. @@ -611,7 +631,7 @@ class _Ref extends Ref { /// As opposed to [listen], the listener will be called even if /// [ProviderElement.updateShouldNotify] returns false, meaning that the previous /// and new value can potentially be identical. - void listenSelf( + RemoveListener listenSelf( void Function(StateT? previous, StateT next) listener, { void Function(Object error, StackTrace stackTrace)? onError, }) { @@ -622,6 +642,11 @@ class _Ref extends Ref { _onErrorSelfListeners ??= []; _onErrorSelfListeners!.add(onError); } + + return () { + _onChangeSelfListeners?.remove(listener); + _onErrorSelfListeners?.remove(onError); + }; } } diff --git a/packages/riverpod/lib/src/framework.dart b/packages/riverpod/lib/src/framework.dart index 7a935e78b..c722c8cd9 100644 --- a/packages/riverpod/lib/src/framework.dart +++ b/packages/riverpod/lib/src/framework.dart @@ -6,6 +6,7 @@ import 'dart:math' as math; import 'package:collection/collection.dart'; import 'package:meta/meta.dart'; +import 'package:state_notifier/state_notifier.dart'; import 'package:test/test.dart' as test; import 'common/env.dart'; import 'common/pragma.dart'; @@ -27,5 +28,4 @@ part 'core/modifiers/select.dart'; part 'core/scheduler.dart'; part 'core/override_with_value.dart'; part 'core/override.dart'; -part 'core/devtool.dart'; part 'core/modifiers/future.dart'; diff --git a/packages/riverpod/test/src/core/provider_container_test.dart b/packages/riverpod/test/src/core/provider_container_test.dart index f73e1c619..ea4c0c8dd 100644 --- a/packages/riverpod/test/src/core/provider_container_test.dart +++ b/packages/riverpod/test/src/core/provider_container_test.dart @@ -81,11 +81,6 @@ TypeMatcher isProviderDirectory({ } void main() { - tearDown(() { - // Verifies that there is no container leak. - expect(DebugRiverpodDevtoolBiding.containers, isEmpty); - }); - group('ProviderPointerManager', () { group('findDeepestTransitiveDependencyProviderContainer', () { final transitiveDependency = Provider( @@ -1025,13 +1020,6 @@ void main() { ); }); - test('registers itself in the container list', () { - final container = ProviderContainer(); - addTearDown(container.dispose); - - expect(DebugRiverpodDevtoolBiding.containers, [container]); - }); - test('throws if "parent" is disposed', () { final root = ProviderContainer(); root.dispose(); @@ -1046,11 +1034,6 @@ void main() { isEmpty, reason: 'Invalid containers should not be added as children', ); - expect( - DebugRiverpodDevtoolBiding.containers, - isEmpty, - reason: 'Invalid containers should not be added to the global list', - ); }); test('if parent is null, assign "root" to "null"', () { @@ -1812,17 +1795,6 @@ void main() { expect(callCount, 0); }); - test('unregister itself from the container list', () { - final container = ProviderContainer(); - addTearDown(container.dispose); - - expect(DebugRiverpodDevtoolBiding.containers, [container]); - - container.dispose(); - - expect(DebugRiverpodDevtoolBiding.containers, isEmpty); - }); - test('Disposes its children first', () { final rootOnDispose = OnDisposeMock(); final childOnDispose = OnDisposeMock(); diff --git a/packages/riverpod/test/src/core/ref_test.dart b/packages/riverpod/test/src/core/ref_test.dart index f3610bfae..1377a78cb 100644 --- a/packages/riverpod/test/src/core/ref_test.dart +++ b/packages/riverpod/test/src/core/ref_test.dart @@ -351,24 +351,6 @@ void main() { }); }); - test("can't use ref inside onDispose", () { - final provider2 = Provider((ref) => 0); - final provider = Provider((ref) { - ref.onDispose(() { - ref.watch(provider2); - }); - return ref; - }); - final container = ProviderContainer.test(); - - container.read(provider); - - final errors = []; - runZonedGuarded(container.dispose, (err, _) => errors.add(err)); - - expect(errors, [isA()]); - }); - group( 'asserts that a provider cannot depend on a provider that is not in its dependencies:', () { @@ -2141,6 +2123,23 @@ void main() { }); group('.onRemoveListener', () { + test('returns a way to unregister the listener', () { + final container = ProviderContainer.test(); + final listener = OnRemoveListener(); + late RemoveListener remove; + final provider = Provider((ref) { + remove = ref.onRemoveListener(listener.call); + }); + + final sub = container.listen(provider, (previous, next) {}); + + remove(); + + sub.close(); + + verifyZeroInteractions(listener); + }); + test('is called on read', () { final container = ProviderContainer.test(); final listener = OnRemoveListener(); @@ -2319,6 +2318,24 @@ void main() { }); group('.onAddListener', () { + test('returns a way to unregister the listener', () { + final container = ProviderContainer.test(); + final listener = OnAddListener(); + late RemoveListener remove; + final provider = Provider((ref) { + remove = ref.onAddListener(listener.call); + }); + + container.listen(provider, (previous, next) {}); + clearInteractions(listener); + + remove(); + + container.listen(provider, (previous, next) {}); + + verifyZeroInteractions(listener); + }); + test('is called on read', () { final container = ProviderContainer.test(); final listener = OnAddListener(); @@ -2484,6 +2501,24 @@ void main() { }); group('.onResume', () { + test('returns a way to unregister the listener', () { + final container = ProviderContainer.test(); + final listener = OnResume(); + late RemoveListener remove; + final provider = Provider((ref) { + remove = ref.onResume(listener.call); + }); + + final sub = container.listen(provider, (previous, next) {}); + + remove(); + + sub.pause(); + sub.resume(); + + verifyZeroInteractions(listener); + }); + test('is not called on initial subscription', () { final container = ProviderContainer.test(); final listener = OnResume(); @@ -2675,6 +2710,23 @@ void main() { }); group('.onCancel', () { + test('returns a way to unregister the listener', () { + final container = ProviderContainer.test(); + final listener = OnCancelMock(); + late RemoveListener remove; + final provider = Provider((ref) { + remove = ref.onCancel(listener.call); + }); + + final sub = container.listen(provider, (previous, next) {}); + + remove(); + + sub.close(); + + verifyZeroInteractions(listener); + }); + test( 'is called when dependent is invalidated and was the only listener', // TODO deal with now that we have onPause @@ -2871,6 +2923,42 @@ void main() { }); group('.onDispose', () { + test('returns a way to unregister the listener', () async { + final container = ProviderContainer.test(); + final listener = OnDisposeMock(); + late RemoveListener remove; + final provider = Provider.autoDispose((ref) { + remove = ref.onDispose(listener.call); + }); + + final sub = container.listen(provider, (previous, next) {}); + + remove(); + + sub.close(); + await container.pump(); + + verifyZeroInteractions(listener); + }); + + test("can't use ref inside onDispose", () { + final provider2 = Provider((ref) => 0); + final provider = Provider((ref) { + ref.onDispose(() { + ref.watch(provider2); + }); + return ref; + }); + final container = ProviderContainer.test(); + + container.read(provider); + + final errors = []; + runZonedGuarded(container.dispose, (err, _) => errors.add(err)); + + expect(errors, [isA()]); + }); + test( 'calls all the listeners in order when the ProviderContainer is disposed', () { diff --git a/packages/riverpod/test/src/matrix.dart b/packages/riverpod/test/src/matrix.dart index 8643c0a61..202f2a010 100644 --- a/packages/riverpod/test/src/matrix.dart +++ b/packages/riverpod/test/src/matrix.dart @@ -1,6 +1,7 @@ import 'dart:async'; import 'package:riverpod/src/internals.dart'; +import 'package:state_notifier/state_notifier.dart'; import 'package:test/test.dart' hide Retry; part 'matrix/async_notifier_provider.dart'; diff --git a/packages/riverpod/test/src/matrix/notifier_provider.dart b/packages/riverpod/test/src/matrix/notifier_provider.dart index dd4299b51..bdc31dc61 100644 --- a/packages/riverpod/test/src/matrix/notifier_provider.dart +++ b/packages/riverpod/test/src/matrix/notifier_provider.dart @@ -126,7 +126,7 @@ abstract class TestNotifier implements $Notifier { set state(StateT value); @override - void listenSelf( + RemoveListener listenSelf( void Function(StateT? previous, StateT next) listener, { void Function(Object error, StackTrace stackTrace)? onError, }); @@ -149,7 +149,7 @@ class DeferredNotifier extends Notifier Ref get ref; @override - void listenSelf( + RemoveListener listenSelf( void Function(StateT? previous, StateT next) listener, { void Function(Object error, StackTrace stackTrace)? onError, }); diff --git a/packages/riverpod/test/src/providers/async_notifier_test.dart b/packages/riverpod/test/src/providers/async_notifier_test.dart index 497fc2609..cd66d250d 100644 --- a/packages/riverpod/test/src/providers/async_notifier_test.dart +++ b/packages/riverpod/test/src/providers/async_notifier_test.dart @@ -439,33 +439,54 @@ void main() { ); }); - test('supports listenSelf', () { - final listener = Listener>(); - final onError = ErrorListener(); - final provider = factory.simpleTestProvider((ref, self) { - self.listenSelf(listener.call, onError: onError.call); - Error.throwWithStackTrace(42, StackTrace.empty); + group('listenSelf', () { + test('can remove the listener', () async { + final container = ProviderContainer.test(); + final listener = Listener>(); + late final RemoveListener remove; + final provider = factory.simpleTestProvider((ref, self) { + remove = self.listenSelf(listener.call); + return 0; + }); + + container.listen(provider.notifier, (previous, next) {}); + clearInteractions(listener); + + remove(); + + container.read(provider.notifier).state = const AsyncData(42); + + verifyZeroInteractions(listener); }); - final container = ProviderContainer.test(); - container.listen(provider, (previous, next) {}); + test('supports listenSelf', () { + final listener = Listener>(); + final onError = ErrorListener(); + final provider = factory.simpleTestProvider((ref, self) { + self.listenSelf(listener.call, onError: onError.call); + Error.throwWithStackTrace(42, StackTrace.empty); + }); + final container = ProviderContainer.test(); - verifyOnly( - listener, - listener(null, const AsyncError(42, StackTrace.empty)), - ); - verifyZeroInteractions(onError); + container.listen(provider, (previous, next) {}); - container.read(provider.notifier).state = const AsyncData(42); + verifyOnly( + listener, + listener(null, const AsyncError(42, StackTrace.empty)), + ); + verifyZeroInteractions(onError); - verifyNoMoreInteractions(onError); - verifyOnly( - listener, - listener( - const AsyncError(42, StackTrace.empty), - const AsyncData(42), - ), - ); + container.read(provider.notifier).state = const AsyncData(42); + + verifyNoMoreInteractions(onError); + verifyOnly( + listener, + listener( + const AsyncError(42, StackTrace.empty), + const AsyncData(42), + ), + ); + }); }); test( diff --git a/packages/riverpod/test/src/providers/notifier_test.dart b/packages/riverpod/test/src/providers/notifier_test.dart index bbd164b7d..0d5dca1b8 100644 --- a/packages/riverpod/test/src/providers/notifier_test.dart +++ b/packages/riverpod/test/src/providers/notifier_test.dart @@ -435,18 +435,54 @@ void main() { expect(container.read(provider.notifier).state, 0); }); - test('supports listenSelf((State? prev, State next) {})', () { - final listener = Listener(); - final onError = ErrorListener(); - final provider = factory.simpleTestProvider((ref, self) { - self.listenSelf(listener.call, onError: onError.call); - Error.throwWithStackTrace(42, StackTrace.empty); + group('listenSelf', () { + test('can remove the data listener', () async { + final container = ProviderContainer.test(); + final listener = Listener(); + late final RemoveListener remove; + final provider = factory.simpleTestProvider((ref, self) { + remove = self.listenSelf(listener.call); + return 0; + }); + + container.listen(provider.notifier, (previous, next) {}); + clearInteractions(listener); + + remove(); + + container.read(provider.notifier).state = 42; + + verifyZeroInteractions(listener); + }); + + test('can remove the error listener', () async { + final container = ProviderContainer.test(); + final listener = ErrorListener(); + final provider = factory.simpleTestProvider((ref, self) { + final remove = self.listenSelf((a, b) {}, onError: listener.call); + remove(); + + throw StateError(''); + }); + + container.listen(provider.notifier, (previous, next) {}); + + verifyZeroInteractions(listener); }); - final container = ProviderContainer.test(); - expect(() => container.read(provider), throwsA(42)); + test('supports listenSelf((State? prev, State next) {})', () { + final listener = Listener(); + final onError = ErrorListener(); + final provider = factory.simpleTestProvider((ref, self) { + self.listenSelf(listener.call, onError: onError.call); + Error.throwWithStackTrace(42, StackTrace.empty); + }); + final container = ProviderContainer.test(); - verifyOnly(onError, onError(42, StackTrace.empty)); + expect(() => container.read(provider), throwsA(42)); + + verifyOnly(onError, onError(42, StackTrace.empty)); + }); }); test('filters state update by identical by default', () { diff --git a/packages/riverpod/test/src/providers/stream_notifier_test.dart b/packages/riverpod/test/src/providers/stream_notifier_test.dart index 6a6f93bc0..d73d4524b 100644 --- a/packages/riverpod/test/src/providers/stream_notifier_test.dart +++ b/packages/riverpod/test/src/providers/stream_notifier_test.dart @@ -360,33 +360,54 @@ void main() { ); }); - test('supports listenSelf', () { - final listener = Listener>(); - final onError = ErrorListener(); - final provider = factory.simpleTestProvider((ref, self) { - self.listenSelf(listener.call, onError: onError.call); - Error.throwWithStackTrace(42, StackTrace.empty); + group('listenSelf', () { + test('can remove the listener', () async { + final container = ProviderContainer.test(); + final listener = Listener>(); + late final RemoveListener remove; + final provider = factory.simpleTestProvider((ref, self) { + remove = self.listenSelf(listener.call); + return Stream.value(42); + }); + + container.listen(provider.notifier, (previous, next) {}); + clearInteractions(listener); + + remove(); + + container.read(provider.notifier).state = const AsyncData(42); + + verifyZeroInteractions(listener); }); - final container = ProviderContainer.test(); - container.listen(provider, (previous, next) {}); + test('supports listenSelf', () { + final listener = Listener>(); + final onError = ErrorListener(); + final provider = factory.simpleTestProvider((ref, self) { + self.listenSelf(listener.call, onError: onError.call); + Error.throwWithStackTrace(42, StackTrace.empty); + }); + final container = ProviderContainer.test(); - verifyOnly( - listener, - listener(null, const AsyncError(42, StackTrace.empty)), - ); - verifyZeroInteractions(onError); + container.listen(provider, (previous, next) {}); - container.read(provider.notifier).state = const AsyncData(42); + verifyOnly( + listener, + listener(null, const AsyncError(42, StackTrace.empty)), + ); + verifyZeroInteractions(onError); - verifyNoMoreInteractions(onError); - verifyOnly( - listener, - listener( - const AsyncError(42, StackTrace.empty), - const AsyncData(42), - ), - ); + container.read(provider.notifier).state = const AsyncData(42); + + verifyNoMoreInteractions(onError); + verifyOnly( + listener, + listener( + const AsyncError(42, StackTrace.empty), + const AsyncData(42), + ), + ); + }); }); test( diff --git a/packages/riverpod/test/src/utils.dart b/packages/riverpod/test/src/utils.dart index 5321322b3..bdabd4732 100644 --- a/packages/riverpod/test/src/utils.dart +++ b/packages/riverpod/test/src/utils.dart @@ -8,6 +8,8 @@ import 'package:test/test.dart' hide Retry; export '../old/utils.dart' show ObserverMock, isProviderObserverContext, isMutationContext; +typedef RemoveListener = void Function(); + List captureErrors(List cb) { final errors = []; for (final fn in cb) {