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

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

Open
eli1stark opened this issue Mar 29, 2024 · 9 comments
Open

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

eli1stark opened this issue Mar 29, 2024 · 9 comments
Assignees
Labels
documentation Improvements or additions to documentation needs triage

Comments

@eli1stark
Copy link

eli1stark commented Mar 29, 2024

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
import 'dart:developer';
import 'package:flutter/material.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';

final appNotipod = NotifierProvider<AppNotifier, int>(AppNotifier.new);
final detailsNotipod = NotifierProvider<DetailsNotifier, int>(DetailsNotifier.new);

class AppNotifier extends Notifier<int> {
  @override
  int build() => 0;
  void increment() => state++;
}

class DetailsNotifier extends Notifier<int> {
  @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?

@eli1stark eli1stark added documentation Improvements or additions to documentation needs triage labels Mar 29, 2024
@rrousselGit
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

eli1stark commented Mar 29, 2024

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
Copy link
Author

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
Copy link
Owner

rrousselGit commented Mar 29, 2024

I would suggest maybe renaming onCancel to onPaused.

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

@eli1stark
Copy link
Author

eli1stark commented Mar 29, 2024

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
Copy link
Owner

, 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation needs triage
Projects
None yet
Development

No branches or pull requests

2 participants