Warn against using ref.watch on a provider's notifier

[songyang-dev]

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

I'm new to Riverpod and I come from Provider. As I was using Riverpod, I thought ref.watch(provider.notifier) was doing the same thing as ref.watch(provider) when it comes to registering the widget for changes. Since the recommended way of triggering a notification to the provider is to use the custom methods declared inside the provider, I thought ref should just watch the provider's notifier to cut down the lines of code.

This leads to the false impression that the widget was watching the provider for changes.

Describe the solution you'd like

1. The linter should warn against watching a provider's notifier.

Describe alternatives you've considered

2. A differently named method of ref should be recommended to access the provider instance's methods.

Additional context
Here is an example code snippet that I tripped over today.

@override Widget build(BuildContext context, WidgetRef ref) {
    var chosenIntervals = ref.watch(freePracticeIntervalsProvider.notifier); // this does not make the widget rebuild 
    var isSelected = chosenIntervals.contains(musicInterval);
}

[martinralfreindl]

Can confirm that this is a very common issue I see. I've onboarded multiple juniors onto riverpod, and they all make this mistake at least once.

[timcreatedit]

In some edge-cases you might want to watch the notifier, however, I agree that this is a super common issue for people who are new to Riverpod, and it should generally be discouraged. Any setup where you don't access a notifier via ref.read whenever you want to use it is also often not the best way to do it I've noticed.

[noriHanda]

Only valid cases when you want to watch notifier that I know of are onPressed: ref.watch(someProvider.notifier).fire and

@override Widget build(BuildContext context, WidgetRef ref) {
    final notifier = ref.watch(someProvider.notifier);
    return Column(
         children: [
                Button(onPressed: () => notifier.fire('boom')),
                Button(onPressed: () => notifier.fire('beep')),
         ]
    );
}

[PollyGlot]

A bit surprised to see this as a warning, as they return different types. I don't see any case to watch a notifier, but we watch the state (AsyncValue) with a provider.

[rrousselGit]

To me, that's more a misuse of the notifier than a misuse of ref.watch
It is part of the good practices that Notifiers should not expose any state outside of its state field.

There's already a warning telling users not to expose public getters/properties in their notifiers. That notifier.contains(...) in the OP is similar to this. It just bypasses the linter because that's not a getter/property.

There are legitimate use-cases for ref.watch(provider.notifier), especially with the 3.0 where a new Notifier will be recreated when Notifier.build is reinvoked.

Typically this is when using tear-offs:

final notifier = ref.watch(provider.notifier);

return Button(onTap: notifier.increment);

If the notifier were to be recreated, it should re-invoke the build method because the increment method changed.

---

I could instead see a warning against using Notifier members within build:

Widget build(ctx) {
  final notifier = ref.watch(p.notifier); // OK, no warning

  notifier.doSomething(); // KO, don't use notifier members inside build
}

[timcreatedit]

final notifier = ref.watch(provider.notifier);

return Button(onTap: notifier.increment);

Wouldn't this also be less desirable than

 return Button(onTap: ref.read(provider.notifier).increment);

due to the unnecessary rebuilds?

[noriHanda]

No, `onTap: ref.read(provider.notifier).increment` breaks “No ref.read in build method” rule.

…

2024/03/27 21:51、Tim ***@***.***>のメール: final notifier = ref.watch(provider.notifier); return Button(onTap: notifier.increment); Wouldn't this also be less desirable than return Button(onTap: ref.read(provider.notifier).increment); due to the unnecessary rebuilds? — Reply to this email directly, view it on GitHub <#3451 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AENYN5FAPUGLCSUBBGZEMGTY2KXDZAVCNFSM6AAAAABFKAHYVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRSGY4TCNZRGA>. You are receiving this because you commented.

[rrousselGit]

Wouldn't this also be less desirable than

 return Button(onTap: ref.read(provider.notifier).increment);

due to the unnecessary rebuilds?

No that's incorrect.
Whatever rebuild we have here by using ref.watch is necessary.

If the notifier instance changed, the button must have its onTap method changed to point to the new notifier.

Otherwise you'd be calling increment on a disposed object.

[timcreatedit]

Oh, so in that case the tear-off points to an outdated function? That's crazy, I thought what I wrote would be equivalent to

return Button(onTap: () => ref.read(provider.notifier).increment());

Or am I crazy and that also wouldn't work?

Unrelated, but unnecessary_lambdas is a bit misleading then.

[rrousselGit]

Nope, both variants aren't the same.
That's a common tear-off / variable dereference topic.

unnecessary_lambdas

The warning doesn't appear if a referenced variable is coming from an unknown source. There won't be such an issue here. Otherwise you would see the warning already when using ref.read :)

[timcreatedit]

The warning doesn't appear if a referenced variable is coming from an unknown source. There won't be such an issue here. Otherwise you would see the warning already when using ref.read :)

That's why I said misleading, not wrong :) I'm gonna go out there and suspect that if you survey all Flutter devs, most would think that tear-offs are just syntactic sugar (in part because in some cases, their linter tells them to put them there), but maybe I'm projecting.

Anyway, thanks for the explanation!

[rrousselGit]

This is technically not coming from tear-offs, but how variables behave.

Consider:

int state = 0;
final previousState = state;
state++;

print('$previousState $state'); // prints "0 1"

That's why you should use ref.watch if you write:

final notifier = ref.watch(provider.notifier);

return Button(onTap: () => notifier.increment());

All of that will be covered by a separate lint that spots watch vs read mistakes.

[songyang-dev]

Hi @rrousselGit, thanks for the clarification! I'll keep in mind the differences between watching the provider and the notifier.

[nateshmbhat]

can you please add a section in the docs clearly explaning this differences with code examples ?

[songyang-dev]

can you please add a section in the docs clearly explaning this differences with code examples ?

Consider starting a new issue for this if there isn't already one.