riverpod_graph: don't crash on: 1) provider is a method of instance of another provider 2) recursive watch calls, like: ref.watch(ref.watch(provider))

[Alexqwesa]

ConsumerWidgetVisitor: store map of local variables, to solve cases there provider is a method of instance of another provider state, i.e.:

final worker = ref.watch(currentWorker); 
final clients = ref.watch(worker.clientsOf) 
//there: 
class Worker{ 
Provider get clients => clientsOfWorker(this); 
// other code....
}

ConsumerWidgetVisitor: allow recursive watch, like:

ref.watch(provider1(ref.watch(provider2)))  // updated

[codecov]

Codecov Report

Merging #2870 (6f1e224) into master (6c9b73c) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2870   +/-   ##
=======================================
  Coverage   95.24%   95.24%           
=======================================
  Files          53       53           
  Lines        2252     2252           
=======================================
  Hits         2145     2145           
  Misses        107      107           

[rrousselGit]

class Worker{ 
Provider get clients => clientsOfWorker(this); 

We could fail more gracefully, but that's not supposed to be supported. Folks shouldn't do this.
Same goes for ref.watch(ref.watch(provider)).

Providers should exclusively be top-level final variables.

[rrousselGit]

First and foremost, this would need testing :)

But given my previous comment, I'm not sure I reasonably want to support this. At best we should have a nice error message I'd say.

In any case, your work is bound to be removed soon enough.
riverpod_graph should move its logic to riverpod_analyzer_utils.

If we want to do such a thing, I'd suggest implementing this in riverpod_analyzer_utils first. Then refactoring riverpod_graph to use it.
But still, we should discuss about whether we truly want to support that; which I'm not sure.

[Alexqwesa]

first, sorry for typo:
ref.watch(ref.watch(provider)) - i mean: ref.watch(provider2(ref.watch(provider1))

Not sure why it's not supported. There are cases where I need to access a specific family member of provider2, and I need to call provider1 first (which I don't actually need)

Providers should exclusively be top-level final variables.

actually, is there a discussion on this topic? (I understand that it may be more challenging to implement, fine if it is the case, but it seems usable.)

and for use cases:

final worker = ref.watch(currentWorker); 
final client = ref.watch(worker.clientsOf).first;
final services = ref.watch(client.servicesOf);

1. When you type a class instance, you can automatically see all its members in the auto-completion menu of the IDE, which is super nice!
2. and is simply more readable compared to:

final worker = ref.watch(currentWorker); 
final client = ref.watch(clients(worker)).first;
final services = ref.watch(services(client));

An alternative proposition is as follows(but no auto-completion here(( ):

final worker = ref.watch(currentWorker); 
final client = ref.watch(clients.of(worker)).first;
final service = ref.watch(services.of(client)).first;
final proofs = ref.watch(proofs.of(service));

[Alexqwesa]

Anyway, thank for good work and good package))

[Alexqwesa]

First and foremost, this would need testing :)

Sorry, did you mean: i should also provide tests? I just used riverpod_graph on my project, and these are the patches that made it work.

Sorry for third patch - i promise, next time i will look into riverpod_analyzer_utils.))

(btw, riverpod_graph didn't crash on my project only if I use all 3 patches)

[rrousselGit]

i mean: ref.watch(provider2(ref.watch(provider1))

That's much better. I agree with this one

[rrousselGit]

actually, is there a discussion on this topic? (I understand that it may be more challenging to implement, fine if it is the case, but it seems usable.)

Outside of possible memory leak, it's straight up not possible when considering codegen – which is the new recommended syntax

[rrousselGit]

Sorry, did you mean: i should also provide tests? I just used riverpod_graph on my project, and these are the patches that made it work.

Yes we need automated tests, to avoid regressions

[Alexqwesa]

Yes we need automated tests, to avoid regressions

ok, next patch will be with tests

btw, the third patch was a fix for this use case:

final serviceProofAtDate =
    Provider.family<(List<Proof>, ProofList), ClientService>(
  (ref, service) {
    final proofList = proofListProvider(
      contractId: service.contractId,
      // another 6 parameters here...
    );

    return ref.watch(proofList);  // local variable proofList
  },
);

[rrousselGit]

Folks shouldn't do this either.

Sure, the graph shouldn't crash. But it would at least warn

All usages of ref.read/watch/listen must be statically analysable. This is important for certain features which rely on static analysis

[rrousselGit]

I'll close this as riverpod_graph needs an effective rewrite and initial release.

This problem should fix itself by switching to riverpod_analyzer_utils.