rrousselGit / riverpod

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

Delaying await on ref.watch(StreamProvider.last) avoids dispose-loop #543

Closed amitkot closed 3 years ago

amitkot commented 3 years ago

In https://github.com/rrousselGit/river_pod/issues/454#issuecomment-823989374 @rrousselGit suggests avoiding a dispose-loop by delaying the call to await on multiple ref.watch(StreamProvider.last) lines:

final userDataProvider = FutureProvider.autoDispose<UserData>((ref) async {
  final user = ref.watch(userProvider.last);
  final myProfile = ref.watch(myProfileProvider.last);
  final mySettings = ref.watch(mySettingsProvider.future);

  return UserData(
    user: await user,
    myProfile: await myProfile,
    mySettings: await mySettings,
  );
});

I ran into the dispose-loop in my app and indeed delaying the call to await seems to avoid the issue, but it is unclear to me why that is the case.

Another thing I tried was returning a StreamProvider (instead of a FutureProvider) and also calling ref.watch(myProfileProvider.stream):

final userDataProvider = StreamProvider.autoDispose<UserData>((ref) async* {
  final user = ref.watch(userProvider.last);
  final myProfileStream = ref.watch(myProfileProvider.stream);
  final mySettings = ref.watch(mySettingsProvider.future);

  await for (final myProvide in myProfileStream) {
    yield UserData(
      user: await user,
      myProfile: await myProfile,
      mySettings: await mySettings,
    );
  }
});

Why does using a StreamProvider cause the dispose-loop and a FutureProvider with the delayed asyncs does not?

rrousselGit commented 3 years ago

The problem when writing:

await ref.watch(foo);
ref.watch(autoDisposeBar);

is that when the provider rebuild, during the await, our provider no longer listens to autoDisposeBar. This can cause autoDisposeBar to be disposed

amitkot commented 3 years ago

In this case we will get a value from the first awaited provider and autoDisposeBar will be disposed and recreated, right?

Why would this lead to a dispose-loop?

rrousselGit commented 3 years ago

Oh, you're right.

That may have been a bug then. But it likely got fixed as part of the v1.0 changes. Do you mind testing with the dev release and see if you still have the issue?

amitkot commented 3 years ago

I didn't get to testing this issue with the dev release, I will try soon (it requires changes to the Widgets, right?).

In the meantime I ran into this issue again in a different project. This works:

final eventItemsViewModelProvider = StreamProvider.autoDispose
    .family<EventItemsViewModel, String>((ref, eventId) async* {
  final eventViewModelStream = ref.watch(eventViewModelProvider(eventId).stream);
  await for (final eventViewModel in eventViewModelStream) {
    final eventItemsStream = ref.watch(_eventItemsProvider(eventId).stream);
    await for (final eventItems in eventItemsStream) {
      yield EventItemsViewModel(
        event: eventViewModel,
      );
    }
  }
});

This is caught in a dispose-loop:

final eventItemsViewModelProvider = StreamProvider.autoDispose
    .family<EventItemsViewModel, String>((ref, eventId) async* {
  final eventViewModelFuture = ref.watch(eventViewModelProvider(eventId).last);

  final eventItemsStream = ref.watch(_eventItemsProvider(eventId).stream);
  await for (final eventItems in eventItemsStream) {
    yield EventItemsViewModel(
      event: await eventViewModelFuture
    );
  }
});

Ideally, I like the flatter version more than I do the nested-await-for one, but it does not work properly.

By the way, is this the recommended way of combining data from two stream providers? Would I be better off merging the streams directly without riverpod?

rrousselGit commented 3 years ago

The flat version is indeed recommended.

Could you share a way to reproduce this that does not depend on your code?

amitkot commented 3 years ago

Update - switching to the 1.0 dev release helped solve the issue.