riverpod icon indicating copy to clipboard operation
riverpod copied to clipboard

Create `ref.onRefresh` and `ref.onInvalidate` so that a provider can ensure it refreshes properly

Open dominic-cot opened this issue 2 years ago • 3 comments

Is your feature request related to a problem? Please describe.

I have run into a problem recently where if a FutureProvider depends on data from other FutureProviders, refreshing the top provider will not result in us getting any new data, as the lower level providers have already emitted a value.

Example:

final dataAProvider = FutureProvider<DataA>((ref) => http.get('/a'));
final dataBProvider = FutureProvider<DataB>((ref) => http.get('/b'));

final pageDataProvider = FutureProvider<PageModel>((ref) async {
   final dataA = await ref.watch(dataAProvider.future));
   final dataB = await ref.watch(dataBProvider.future));

   return PageModel(dataA, dataB);
});

I would like to be able to refresh the data required by my page by simply calling ref.refresh(pageDataProvider).

Describe the solution you'd like

If, similar to ref.onDispose, we had a ref.onRefresh, we could within a provider say what happens when the provider is refreshed, so that it can ensure it refreshes all of the data it needs.

In my example from above it would look something like this:

final pageDataProvider = FutureProvider<PageModel>((ref) async {
   ref.onRefresh(() {
      ref.refresh(dataAProvider);
      ref.refresh(dataBProvider);
   });

   final dataA = await ref.watch(dataAProvider.future));
   final dataB = await ref.watch(dataBProvider.future));

   return PageModel(dataA, dataB);
});

Describe alternatives you've considered

To get around this for now, I have created separate refreshPageDataProvider that I call instead of being able to just refresh the provider.

Update: Only just noticed this open PR

  • Any idea when this will be completed?
  • Any plans on also adding an onInvalidate?

dominic-cot avatar May 12 '22 10:05 dominic-cot

Hi,

what is onInvalidate? I'm not sure I've understood.

Anyways I'm kinda "meh" on the onRefresh proposal used to solve your problem; this would encourage implementing side-effects on the refresh event, which could be unwanted in the future (I see this as a maintainability problem). But maybe it is just me.

Maybe -as a proposal- if would make sense to introduce a refreshDeep directive that recursively refreshes every provider involved in the evaluation. That's more a more maintainable method, you can clearly see what refreshDeep would do.

lucavenir avatar May 12 '22 13:05 lucavenir

Hi @lucavenir , thanks for your response!

The idea would be for onInvalidate to work for ref.invalidate the same way as onRefresh does for ref.refresh in the PR that I linked.

I'm not sure I see the maintainability problem. The onRefresh method would live inside of the provider, so it is very clear what a provider does when it is created, and also what it does when it is refreshed.

Interesting suggestion for refreshDeep. Only problem I see with that is that often there are things you don't want to refresh, so we would still have no control over this.

dominic-cot avatar May 13 '22 15:05 dominic-cot

A refreshDeep would be a big no-no.

Honestly I'd still a bit on the fence about this onRefresh life-cycle. I'm not sure whether it's really do its job. Say someone does ref.invalidate(provider), that life-cycle wouldn't be triggered.

And onInvalidate is a bit no-no too, as "invalidate" is triggered in various situations implicitly. It's not a safe life-cycle to perform side-effects such as refreshing other providers.

With the upcoming AsyncNotifier, you'd be able to have a FutureProvider which defines methods. So you could define a custom "refresh" function:

@riverpod
class MyClass extends _$MyClass {
  @override
  Future<Model> build() => ...

  void refresh() {
    ref.invalidateSelf();
    ref.invalidate(otherProvider);
  }
}


...

ref.read(myClassProvider.notifier).refresh();

So the value for this would be low. I'm not sure whether it's worth the possible confusion/dangers

rrousselGit avatar Sep 18 '22 17:09 rrousselGit

A refreshDeep would be a big no-no.

Honestly I'd still a bit on the fence about this onRefresh life-cycle. I'm not sure whether it's really do its job. Say someone does ref.invalidate(provider), that life-cycle wouldn't be triggered.

And onInvalidate is a bit no-no too, as "invalidate" is triggered in various situations implicitly. It's not a safe life-cycle to perform side-effects such as refreshing other providers.

With the upcoming AsyncNotifier, you'd be able to have a FutureProvider which defines methods. So you could define a custom "refresh" function:

@riverpod
class MyClass extends _$MyClass {
  @override
  Future<Model> build() => ...

  void refresh() {
    ref.invalidateSelf();
    ref.invalidate(otherProvider);
  }
}


...

ref.invalidate(myClassProvider.notifier).refresh();

So the value for this would be low. I'm not sure whether it's worth the possible confusion/dangers

is there a way that i can catch error during refresh base on this example?

let's say everything works fine in the first build() method, and the people trying pull down to refresh page which invoking function you defined refresh().

at this moment something could be wrong during server, but the method ref.invalidateSelf() has void return so i cannot catch the error in this case.

void refresh() {
  try {
      ref.invalidateSelf();
      ref.invalidate(otherProvider);
      
     return IndicatorResult.success; 
  } catch (err) {
    IndicatorResult.fail;
  }
}

iEricKoh avatar Nov 14 '22 06:11 iEricKoh

I don't plan on adding ways to react to refresh/invalidate quite yet, so closing for now.

rrousselGit avatar Jun 15 '23 10:06 rrousselGit