linter
linter copied to clipboard
dart-lang
proposal: extend use_build_context_synchronously to consider Riverpod's WidgetRef
Hello! I'm not sure whether this is reasonable, but it's worth asking.
Long story short, Riverpod would like to define a lint strictly identical to use_build_context_synchronously but for its WidgetRef object.
The lint would be identical in every point, such that BuildContext/WidgetRef can constantly be interchanged. As a matter of fact, WidgetRef is a BuildContext. It uses a different interface to cause less confusion and allows folks using package:provider to smoothly migrate to package:riverpod.
As such, would it be reasonable to update use_build_context_synchronously to handle Riverpod's WidgetRef too?
Such that we have:
void fn() async {
WidgetRef ref;
await something;
ref.anything();
^^^^^^^^^^ use_build_context_synchronously
if (mounted) {
ref.anything(); // ok
}
I understand that this repository probably wants to be clean of third-party package logic.
If necessary, maybe a solution could be to add a custom package:meta annotation that both WidgetRef and BuildContext would use (@shouldUseSynchronously or @isBuildContext?)
The current rule says
When a
BuildContextis used from aStatefulWidget, themountedproperty must be checked after an asynchronous gap.
Does the same apply for WidgetRef, or would we need some way for the annotation to specify a property that can be checked?
It's probably worth noting that WidgetRef is a class that interacts with BuildContext (hence why the rules should be respected)
Such that we have:
abstract class WidgetRef {
void doSomething() {
Object.of(context).doSomething();
}
}
So maybe what makes sense would be to annotate the functions of WidgetRef, not WidgetRef itself. Maybe something like @useBuildContext?
abstract class WidgetRef {
@useBuildContext
void doSomething();
}
Definitely worth considering.
Would this be useful for other users? I don't know Flutter well enough to know whether users might write methods similar to doSomething or whether that's unlikely to occur very often.
It's probably not very common. Most cases would end-up passing the context variable as parameter to their function
The case described here is different because it already has access to the context, so passing it is redundant
For what it's worth, I'm working on riverpod-specific lint-rules using analyzer_plugin
So I could implement this on my own if necessary. Although the annotation approach here would make my like a lot easier
@rrousselGit Another use case would be the mounted property on StateNotifier.
@bwilkerson Perhaps we can generalize the rule to a bool get mounted property regardless of the class?
I.e. if the class has such a property (I assume that we hard-reference to the name mounted here) and we don't add a return after an await, we flag the block of code?
class Foo {
bool get mounted => true;
void qux() {}
void bar() async {
await Future.delayed(const Duration(seconds: 1));
if(!mounted) { // LINT without this check
return;
}
qux();
}
}
I'd like to get some input from the Flutter team before making a decision, but I'd much rather see us define an annotation that can be used outside of Flutter rather than implement some special case logic.
I agree, having the use_build_context_synchronously lint is good in a Flutter context, but the idea behind it (not using some state that might have become invalid after an async gap) should be the focus of the lint imo.