-
-
Notifications
You must be signed in to change notification settings - Fork 171
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
Reduce boilerplate code of controller (ChangeNotifier) hooks #415
base: master
Are you sure you want to change the base?
Conversation
That looks interesting. A few things:
|
/// ``` | ||
T useChangeNotifier<T extends ChangeNotifier>( | ||
T Function() factory, [ | ||
String? label, |
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 like this being part of the public API
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.
as I commented, it can be used to hook a common ChangeNotifier that has no hook yet, for example,CupertinoTabController
which was expected by some people:
#222
|
No but then we should mark those as |
Not if we export those hooks. |
👋 |
Let's recap:
I prefer to export them, if you think it is proper, I will add test cases for them. |
If we want to export them, I'll also be a bit more strict on the API. |
which restriction do you want to change? I will change it. |
I think we need to export them, to reduce these boring code: final controller = useMemoized(
() => SomeChangeNotifier(),
);
useEffect(
() => () {
controller.dispose();
},
[controller],
); |
I don't like the API of that If we want a reusable hook for this, I'd like a more Riverpod-like API. final model = useValue((ref) {
// Offer various life-cycles
ref.onDispose(...);
ref.state = ...;
ref.notifyListeners();
return MyModel();
}, [keys]); This merges various hooks in a single API. |
If this PR is merged, I will group all of them into a single file, making the repo cleaner.