rrousselGit / riverpod

A reactive caching and data-binding framework. Riverpod makes working with asynchronous code a breeze.
https://riverpod.dev
MIT License
5.82k stars 888 forks source link

docs: Providers are not paused properly if no longer listened. #3460

Open eli1stark opened 1 month ago

eli1stark commented 1 month ago

hooks_riverpod: v2.5.1

Describe what scenario you think is uncovered by the existing examples/articles Providers are not paused when no longer listened as stated in the docs but they can be disposed if .autoDispose is specified.

Describe why existing examples/articles do not cover this case Source I understand that Riverpod is currently undergoing a major rewrite, and I'm aware that some parts of the documentation may be outdated. However, it seems to me that this concept forms the backbone of how Provider operates and doesn't appear to have changed or will change. Is this correct?

From the docs:

When an Alive provider is no longer listened to by other providers or the UI, it goes into a Paused state. This means that it no longer will react to changes on providers it is listening to.

Additional context I created a reproducible example, when you press "Push Details" then "Pop Details" and start updating appNotipod, listener will be triggered inside detailsNotipod but it shouldn't as stated in the docs because it must be paused.

Because of the way lifecycle of providers, workarounds like EagerInitialization exist, we need to watch for providers at the root of the app so they are not paused, but if this is not the case and providers are not paused when no longer listened we don't need EagerInitialization.

Example ```dart import 'dart:developer'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; final appNotipod = NotifierProvider(AppNotifier.new); final detailsNotipod = NotifierProvider(DetailsNotifier.new); class AppNotifier extends Notifier { @override int build() => 0; void increment() => state++; } class DetailsNotifier extends Notifier { @override int build() { ref.listen(appNotipod, (previous, next) { log('Listen appNotipod ${DateTime.now()}'); }); return 0; } void increment() => state++; } void main() => runApp(const ProviderScope(child: App())); class App extends StatelessWidget { const App({super.key}); @override Widget build(BuildContext context) { return const MaterialApp( home: Home(), ); } } class Home extends ConsumerWidget { const Home({super.key}); @override Widget build(BuildContext context, WidgetRef ref) { final app = ref.watch(appNotipod); final appN = ref.watch(appNotipod.notifier); return Scaffold( backgroundColor: Colors.white, body: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ Center( child: Text('Value: $app'), ), TextButton( onPressed: () => pushDetails(context), child: const Text('Push Details'), ), TextButton( onPressed: appN.increment, child: const Text('Update'), ), ], ), ); } } class Details extends ConsumerWidget { const Details({super.key}); @override Widget build(BuildContext context, WidgetRef ref) { final details = ref.watch(detailsNotipod); final detailsN = ref.watch(detailsNotipod.notifier); return Scaffold( backgroundColor: Colors.white, body: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ Center( child: Text('Value: $details'), ), TextButton( onPressed: () => Navigator.pop(context), child: const Text('Pop Details'), ), TextButton( onPressed: detailsN.increment, child: const Text('Update'), ), ], ), ); } } void pushDetails(BuildContext context) { Navigator.push( context, MaterialPageRoute( builder: (context) { return const Details(); }, ), ); } ```

Is it a bug or outdated docs?

rrousselGit commented 1 month ago

I think you're confused about what "the provider is paused" means. It's about how ref.watch will not cause the provider to rebuild.

For instance consider:

final provider = Provider((ref) {
   print('rebuild');
  Timer(Duration(seconds: 2), () => ref.invalidateSelf());
});

This provider will print rebuild every 2 seconds if it is not paused.

Yet you'll notice that if you do a simple ref.read(provider) and never ref.listen/ref.watch it anywhere, only a single rebuild will be logged.

rrousselGit commented 1 month ago

In your snippet, the provider is correctly paused. This can be checked with ref.onResume/ref.onCancel:

class DetailsNotifier extends Notifier<int> {
  @override
  int build() {
    print('start');
    ref.onResume(() {
      print('resume');
    });
    ref.onCancel(() {
      print('pause');
    });
   ...
  }
}

You'll see that when popping the detail page, pause is called. And reopening it calls resume.

Your specific issue is: ref.listen subscriptions are not cancelled when a provider is paused. So the listener is still invoked, even though the provider is paused. This is voluntary, as paused providers likely want to keep performing work in some cases.

The solution in your case is to manually cancel the subscription:

ProviderSubscription? sub = ref.listen(appNotipod, (previous, next) {
  print('Listen appNotipod  ${DateTime.now()}');
});

// Stop listening on pause
ref.onCancel(() => sub?.cancel());

// Restart the listening on resume
ref.onResume(() => sub = ref.listen(appNotipod, (previous, next) {
  print('Listen appNotipod  ${DateTime.now()}');
})); 

v3 will make this more straightforward as you'll be able to do:

final sub = ref.listen(appNotipod, (previous, next) {
  print('Listen appNotipod  ${DateTime.now()}');
});

ref.onCancel(sub.pause);

// Restart the listening on resume
ref.onResume(sub.resume); 
rrousselGit commented 1 month ago

We could consider making a breaking change for 3.0, and have ref.listen pause by default when a provider is paused. With maybe an optional flag on ref.listen to opt out:

ref.listen(p, maintainSubscriptionOnCancel: true, ...);
rrousselGit commented 1 month ago

In any case, the pausing mechanism is receiving a big overhaul in 3.0. So that docs page will likely receive a lot of change for 3.0 :)

eli1stark commented 1 month ago

I see, so the pausing relates more to the body of the provider. If it just returns a complex object like ProfileService or ProfileNotifier that has its inner subscriptions, they will continue to work.

I would suggest maybe renaming onCancel to onPaused. While I understood what onResume did at first glance, I couldn't guess the purpose of onCancel until I tried it. This naming will mimic lifecycle of the widgets.

Thank you for the clarification!

eli1stark commented 1 month ago

We could consider making a breaking change for 3.0, and have ref.listen pause by default when a provider is paused. With maybe an optional flag on ref.listen to opt out:

ref.listen(p, maintainSubscriptionOnCancel: true, ...);

This would be an interesting concept and would offer more control over background services held by providers.

rrousselGit commented 1 month ago

I would suggest maybe renaming onCancel to onPaused.

I named it onCancel because that's the official name in Dart. Cf StreamController.onCancel

eli1stark commented 1 month ago

I would suggest maybe renaming onCancel to onPaused.

I named it onCancel because that's the official name in Dart. Cf StreamController.onCancel

It makes sense, but aren't we unable to resume a StreamSubscription that has already been canceled? This is where the confusion arises because 'cancel' is more associated with 'dispose' for me, while 'pause' suggests that it's still here and can be resumed later. Similar to StreamSubscription, we can pause them and resume later, but once canceled, it's done.

rrousselGit commented 1 month ago

, but aren't we unable to resume a StreamSubscription that has already been canceled?

No we can. There's "StreamSubscription.resume" and the controller.onResume lifecycle

And "dispose" would be more like controller.onDone