-
-
Notifications
You must be signed in to change notification settings - Fork 956
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
[riverpod_lint] Add new lint incorrect_usage_of_ref_method #2947
Changes from 1 commit
ccfa40d
84ffbd1
005210e
551cb75
1603cad
b2e7ed7
8b426ae
e000d59
22a5ecf
1f58680
e8741ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,7 @@ Riverpod_lint adds various warnings with quick fixes and refactoring options, su | |
- [avoid\_ref\_inside\_state\_dispose](#avoid_ref_inside_state_dispose) | ||
- [notifier\_build (riverpod\_generator only)](#notifier_build-riverpod_generator-only) | ||
- [async\_value\_nullable\_patttern](#async_value_nullable_patttern) | ||
- [incorrect\_usage\_of\_ref\_method](#incorrect_usage_of_ref_method) | ||
- [All assists](#all-assists) | ||
- [Wrap widgets with a `Consumer`](#wrap-widgets-with-a-consumer) | ||
- [Wrap widgets with a `ProviderScope`](#wrap-widgets-with-a-providerscope) | ||
|
@@ -643,6 +644,91 @@ switch (...) { | |
} | ||
``` | ||
|
||
### incorrect_usage_of_ref_method | ||
|
||
Warn if the `ref.watch`/`ref.read`/`ref.listen`/`ref.listenManual` methods are used incorrectly. | ||
|
||
**Good**: | ||
|
||
```dart | ||
final fnProvider = Provider<int>((ref) { | ||
ref.watch(provider); | ||
ref.listen(provider, ...); | ||
return 0; | ||
}); | ||
|
||
class MyNotifier extends Notifier<int> { | ||
int get _readGetter => ref.read(provider); | ||
|
||
@override | ||
int build() { | ||
ref.watch(provider); | ||
ref.listen(provider, ...); | ||
return 0; | ||
} | ||
|
||
void someMethod() { | ||
ref.read(provider); | ||
} | ||
} | ||
|
||
void initState() { | ||
ref.listenManual(provider, ...); | ||
} | ||
|
||
Widget build(ctx, ref) { | ||
ref.watch(provider); | ||
ref.listen(provider, ...); | ||
|
||
return Button( | ||
onPressed: () { | ||
ref.read(provider); | ||
ref.listenManual(provider, ...); | ||
} | ||
); | ||
} | ||
``` | ||
|
||
**Bad**: | ||
|
||
```dart | ||
final fnProvider = Provider<int>((ref) { | ||
ref.read(provider); // use watch instead | ||
return 0; | ||
}); | ||
|
||
class MyNotifier extends Notifier<int> { | ||
int get _watchGetter => ref.watch(provider); // use read instead | ||
|
||
@override | ||
int build() { | ||
ref.read(provider); // use watch instead | ||
return 0; | ||
} | ||
|
||
void someMethod() { | ||
ref.watch(provider); // use read instead | ||
ref.listen(provider, ...); // place listen in build instead | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The recommendation would be to use listenManual There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but this is inside a Notifier, ref is not WidgetRef. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I missed that. The there's no issue with calling listen in methods. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. So this is a valid use case of Ref.listen. void someMethod(){
sub?.close();
sub = ref.listen(provider, ...);
} |
||
} | ||
} | ||
|
||
void initState() { | ||
ref.listen(provider, ...); // use listenManual instead | ||
} | ||
|
||
Widget build(ctx, ref) { | ||
ref.read(provider); // use watch instead | ||
ref.listenManual(provider, ...); // use listen instead | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no strong requirement against this ... as long as they use the returned subscription and deal with it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For instance they could do sub?.cancel();
sub = ref.listenManual(...) |
||
|
||
return Button( | ||
onPressed: () { | ||
ref.watch(provider); // use read instead | ||
ref.listen(provider, ...); // use listenManual instead | ||
} | ||
); | ||
} | ||
``` | ||
|
||
## All assists | ||
|
||
### Wrap widgets with a `Consumer` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recommend making such getters at all. They can introduce bugs implicitly, because the getter may be used in multiple places where some places want
read
and others wantwatch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I will remove it.