diff --git a/packages/riverpod/CHANGELOG.md b/packages/riverpod/CHANGELOG.md index 98f947043..445c7f3c0 100644 --- a/packages/riverpod/CHANGELOG.md +++ b/packages/riverpod/CHANGELOG.md @@ -1,3 +1,8 @@ +## Unreleased patch + +- Fix `ProviderObserver.didUpdateProvider` being called with an incorrect + "provider" parameter when the provider is overridden. + ## 2.4.6 - 2023-11-13 - Exceptions in asynchronous providers are now correctly received diff --git a/packages/riverpod/lib/src/framework/container.dart b/packages/riverpod/lib/src/framework/container.dart index ea68a7ad8..093888cde 100644 --- a/packages/riverpod/lib/src/framework/container.dart +++ b/packages/riverpod/lib/src/framework/container.dart @@ -704,6 +704,9 @@ final b = Provider((ref) => ref.watch(a), dependencies: [a]); ); } } + + @override + String toString() => 'ProviderContainer#$hashCode'; } /// An object that listens to the changes of a [ProviderContainer]. diff --git a/packages/riverpod/lib/src/framework/element.dart b/packages/riverpod/lib/src/framework/element.dart index 6ce2650a5..11ad92401 100644 --- a/packages/riverpod/lib/src/framework/element.dart +++ b/packages/riverpod/lib/src/framework/element.dart @@ -67,7 +67,7 @@ abstract class ProviderElementBase implements Ref, Node { /// The [ProviderContainer] that owns this [ProviderElementBase]. @override ProviderContainer get container => _container; - late ProviderContainer _container; + late final ProviderContainer _container; /// Whether this [ProviderElementBase] is currently listened to or not. /// @@ -573,7 +573,7 @@ The provider ${_debugCurrentlyBuildingElement!.origin} modified $origin while bu for (final observer in _container.observers) { runQuaternaryGuarded( observer.didUpdateProvider, - provider, + origin, previousState, newState.stateOrNull, _container, @@ -586,7 +586,7 @@ The provider ${_debugCurrentlyBuildingElement!.origin} modified $origin while bu error: (newState) { runQuaternaryGuarded( observer.providerDidFail, - provider, + origin, newState.error, newState.stackTrace, _container, diff --git a/packages/riverpod/test/framework/provider_observer_test.dart b/packages/riverpod/test/framework/provider_observer_test.dart index 173685380..b1586435e 100644 --- a/packages/riverpod/test/framework/provider_observer_test.dart +++ b/packages/riverpod/test/framework/provider_observer_test.dart @@ -69,49 +69,48 @@ void main() { verifyNever(observer.providerDidFail(any, any, any, any)); }); - // test( - // 'on scoped ProviderContainer, applies both child and ancestors observers', - // () { - // final provider = StateNotifierProvider, int>( - // (ref) => StateController(0), - // ); - // final observer = ObserverMock(); - // final observer2 = ObserverMock(); - // final observer3 = ObserverMock(); - // final root = createContainer(observers: [observer]); - // final mid = createContainer( - // parent: root, - // observers: [observer2], - // ); - // final child = createContainer( - // parent: mid, - // overrides: [provider.overrideWithValue(StateController(42))], - // observers: [observer3], - // ); - - // expect(child.read(provider), 42); - // expect(mid.read(provider), 0); - - // clearInteractions(observer3); - // clearInteractions(observer2); - // clearInteractions(observer); - - // child.read(provider.notifier).state++; - - // verifyInOrder([ - // observer3.didUpdateProvider(provider, 42, 43, child), - // observer2.didUpdateProvider(provider, 42, 43, child), - // observer.didUpdateProvider(provider, 42, 43, child), - // ]); - - // mid.read(provider.notifier).state++; - - // verify(observer.didUpdateProvider(provider, 0, 1, root)).called(1); - - // verifyNoMoreInteractions(observer3); - // verifyNoMoreInteractions(observer2); - // verifyNoMoreInteractions(observer); - // }); + test( + 'on scoped ProviderContainer, applies both child and ancestors observers', + () { + final provider = StateNotifierProvider, int>( + (ref) => StateController(0), + ); + final observer = ObserverMock('a'); + final observer2 = ObserverMock('b'); + final observer3 = ObserverMock('c'); + + final root = createContainer(observers: [observer]); + final mid = createContainer( + parent: root, + observers: [observer2], + ); + final child = createContainer( + parent: mid, + overrides: [provider.overrideWith((ref) => StateController(42))], + observers: [observer3], + ); + + expect(child.read(provider), 42); + expect(mid.read(provider), 0); + + clearInteractions(observer3); + clearInteractions(observer2); + clearInteractions(observer); + + expect(child.read(provider.notifier).state++, 42); + + verify(observer.didUpdateProvider(provider, 42, 43, child)).called(1); + verify(observer2.didUpdateProvider(provider, 42, 43, child)).called(1); + verify(observer3.didUpdateProvider(provider, 42, 43, child)).called(1); + + mid.read(provider.notifier).state++; + + verify(observer.didUpdateProvider(provider, 0, 1, root)).called(1); + + verifyNoMoreInteractions(observer3); + verifyNoMoreInteractions(observer2); + verifyNoMoreInteractions(observer); + }); test('handles computed provider update', () async { final observer = ObserverMock(); @@ -150,84 +149,76 @@ void main() { ).called(1); }); - // test('didUpdateProviders', () { - // final observer = ObserverMock(); - // final observer2 = ObserverMock(); - // final provider = StateNotifierProvider((_) => Counter()); - // final counter = Counter(); - // final container = createContainer( - // overrides: [ - // provider.overrideWithValue(counter), - // ], - // observers: [observer, observer2], - // ); - // final listener = Listener(); - - // container.listen(provider, listener, fireImmediately: true); - - // verify(listener(null, 0)).called(1); - // verifyNoMoreInteractions(listener); - // verifyInOrder([ - // observer.didAddProvider( - // provider.notifier, - // counter, - // container, - // ), - // observer2.didAddProvider(provider.notifier, counter, container), - // observer.didAddProvider(provider, 0, container), - // observer2.didAddProvider(provider, 0, container), - // ]); - // verifyNoMoreInteractions(observer); - // verifyNoMoreInteractions(observer2); - - // counter.increment(); - - // verifyInOrder([ - // listener(0, 1), - // observer.didUpdateProvider(provider, 0, 1, container), - // observer2.didUpdateProvider(provider, 0, 1, container), - // ]); - // verifyNoMoreInteractions(listener); - // verifyNoMoreInteractions(observer); - // verifyNoMoreInteractions(observer2); - // }); - - // test('guards didUpdateProviders', () { - // final observer = ObserverMock(); - // when(observer.didUpdateProvider(any, any, any, any)) - // .thenThrow('error1'); - // final observer2 = ObserverMock(); - // when(observer2.didUpdateProvider(any, any, any, any)) - // .thenThrow('error2'); - // final observer3 = ObserverMock(); - // final provider = StateNotifierProvider((_) => Counter()); - // final counter = Counter(); - // final container = createContainer( - // overrides: [ - // provider.overrideWithValue(counter), - // ], - // observers: [observer, observer2, observer3], - // ); - - // container.read(provider); - - // clearInteractions(observer); - // clearInteractions(observer2); - // clearInteractions(observer3); - - // final errors = []; - // runZonedGuarded(counter.increment, (err, stack) => errors.add(err)); - - // expect(errors, ['error1', 'error2']); - // verifyInOrder([ - // observer.didUpdateProvider(provider, 0, 1, container), - // observer2.didUpdateProvider(provider, 0, 1, container), - // observer3.didUpdateProvider(provider, 0, 1, container), - // ]); - // verifyNoMoreInteractions(observer); - // verifyNoMoreInteractions(observer2); - // verifyNoMoreInteractions(observer3); - // }); + test('didUpdateProviders', () { + final observer = ObserverMock('a'); + final observer2 = ObserverMock('b'); + final provider = StateNotifierProvider((_) => Counter()); + final counter = Counter(); + final container = createContainer( + overrides: [ + provider.overrideWith((ref) => counter), + ], + observers: [observer, observer2], + ); + final listener = Listener(); + + container.listen(provider, listener.call, fireImmediately: true); + + verify(listener(null, 0)).called(1); + verifyNoMoreInteractions(listener); + verifyInOrder([ + observer.didAddProvider(provider, 0, container), + observer2.didAddProvider(provider, 0, container), + ]); + verifyNoMoreInteractions(observer); + verifyNoMoreInteractions(observer2); + + counter.increment(); + + verifyInOrder([ + listener(0, 1), + observer.didUpdateProvider(provider, 0, 1, container), + observer2.didUpdateProvider(provider, 0, 1, container), + ]); + verifyNoMoreInteractions(listener); + verifyNoMoreInteractions(observer); + verifyNoMoreInteractions(observer2); + }); + + test('guards didUpdateProviders', () { + final observer = ObserverMock(); + when(observer.didUpdateProvider(any, any, any, any)) + .thenThrow('error1'); + final observer2 = ObserverMock(); + when(observer2.didUpdateProvider(any, any, any, any)) + .thenThrow('error2'); + final observer3 = ObserverMock(); + final provider = StateNotifierProvider((_) => Counter()); + final counter = Counter(); + final container = createContainer( + overrides: [provider.overrideWith((ref) => counter)], + observers: [observer, observer2, observer3], + ); + + container.read(provider); + + clearInteractions(observer); + clearInteractions(observer2); + clearInteractions(observer3); + + final errors = []; + runZonedGuarded(counter.increment, (err, stack) => errors.add(err)); + + expect(errors, ['error1', 'error2']); + verifyInOrder([ + observer.didUpdateProvider(provider, 0, 1, container), + observer2.didUpdateProvider(provider, 0, 1, container), + observer3.didUpdateProvider(provider, 0, 1, container), + ]); + verifyNoMoreInteractions(observer); + verifyNoMoreInteractions(observer2); + verifyNoMoreInteractions(observer3); + }); test("Computed don't call didUpdateProviders when value doesn't change", () async { @@ -288,6 +279,69 @@ void main() { }); group('providerDidFail', () { + test( + 'on scoped ProviderContainer, applies both child and ancestors observers', + () async { + final dep = StateProvider((ref) => 0); + final provider = StateNotifierProvider, int>( + (ref) => StateController(0), + ); + final observer = ObserverMock('a'); + final observer2 = ObserverMock('b'); + final observer3 = ObserverMock('c'); + + final root = createContainer(observers: [observer]); + final mid = createContainer( + parent: root, + observers: [observer2], + ); + final child = createContainer( + parent: mid, + overrides: [ + provider.overrideWith((ref) { + if (ref.watch(dep) != 0) { + Error.throwWithStackTrace('error', StackTrace.empty); + } + return StateController(42); + }) + ], + observers: [observer3], + ); + + child.listen(provider, (_, __) {}, onError: (_, __) {}); + + expect(child.read(provider), 42); + expect(mid.read(provider), 0); + + clearInteractions(observer3); + clearInteractions(observer2); + clearInteractions(observer); + + child.read(dep.notifier).state++; + await child.pump(); + + verifyInOrder([ + observer.didDisposeProvider(provider, child), + observer.didUpdateProvider(dep, 0, 1, root), + observer.didUpdateProvider(provider, 42, null, child), + observer.providerDidFail(provider, 'error', StackTrace.empty, child), + ]); + verifyInOrder([ + observer2.didDisposeProvider(provider, child), + observer2.didUpdateProvider(provider, 42, null, child), + observer2.providerDidFail(provider, 'error', StackTrace.empty, child), + ]); + verifyInOrder([ + observer3.didDisposeProvider(provider, child), + observer3.didUpdateProvider(provider, 42, null, child), + observer3.providerDidFail(provider, 'error', StackTrace.empty, child), + ]); + + verifyNoMoreInteractions(observer3); + verifyNoMoreInteractions(observer2); + verifyNoMoreInteractions(observer); + }); + test('is called when FutureProvider emits an error', () async { final observer = ObserverMock(); final container = createContainer(observers: [observer]); diff --git a/packages/riverpod/test/utils.dart b/packages/riverpod/test/utils.dart index 33b4bbe23..745864173 100644 --- a/packages/riverpod/test/utils.dart +++ b/packages/riverpod/test/utils.dart @@ -176,6 +176,15 @@ class _EqualsIgnoringHashCodes extends Matcher { } class ObserverMock extends Mock implements ProviderObserver { + ObserverMock([this.label]); + + final String? label; + + @override + String toString() { + return label ?? super.toString(); + } + @override void didDisposeProvider( ProviderBase? provider,