add simple example on how to listen other providers #3871
Conversation
WalkthroughThe pull request introduces a new documentation file demonstrating a Flutter application using Riverpod for state management. The example showcases how two notifier classes, Changes
Sequence DiagramsequenceDiagram
participant UI as MyHomePage
participant SecondsNotifer
participant MinutesNotifer
UI->>SecondsNotifer: Increment Seconds
alt Seconds reaches 60
SecondsNotifer-->>SecondsNotifer: Reset to 0
SecondsNotifer->>MinutesNotifer: Trigger Minute Increment
end
MinutesNotifer-->>UI: Update Minute State
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
website/docs/essentials/listen_other_providers.mdx (3)
25-44: Simplify the increment logicThe increment logic can be simplified for better readability and maintainability.
class MinutesNotifer extends Notifier<int> { @override int build() { ref.listen(secondProvider, (prev, next) { if (prev == 59 && next == 0) { increment(); } }); return 0; } void increment() { - var nextState = state; - nextState = nextState + 1; - if (nextState == 60) { - nextState = 0; - } - state = nextState; + state = (state + 1) % 60; } }
82-124: Fix typo and improve text clarity
- The variable name
secondsNotifercontains the same typo.- The UI text could be more descriptive for documentation purposes.
- var secondsNotifer = ref.watch(secondProvider.notifier); + var secondsNotifier = ref.watch(secondProvider.notifier); var nbMinutes = ref.watch(minutesProvider); return Scaffold( appBar: AppBar( backgroundColor: Theme.of(context).colorScheme.inversePrimary, - title: const Text('The manual Clock'), + title: const Text('Provider Listening Example'), ), body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: <Widget>[ const Text( - 'press the button to add seconds to the clock:', + 'Tap the + button to increment seconds:', ), Text( '${ref.watch(secondProvider)}', style: Theme.of(context).textTheme.headlineMedium, ), if (nbMinutes != 0) const Text( - 'number of minutes:', + 'Minutes elapsed:', ),
17-23: Add documentation comments for educational purposesSince this is a documentation example, consider adding explanatory comments to help beginners understand the code better.
+/// Manages the minutes state and automatically increments when seconds reset final minutesProvider = NotifierProvider<MinutesNotifer, int>(() { return MinutesNotifer(); }); +/// Manages the seconds state which can be manually incremented final secondProvider = NotifierProvider<SecondsNotifer, int>(() { return SecondsNotifer(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/docs/essentials/listen_other_providers.mdx(1 hunks)
🔇 Additional comments (2)
website/docs/essentials/listen_other_providers.mdx (2)
1-11: LGTM! Imports and metadata are properly structured.
66-80: LGTM! Standard MaterialApp setup.
| final minutesProvider = NotifierProvider<MinutesNotifer, int>(() { | ||
| return MinutesNotifer(); | ||
| }); | ||
|
|
||
| final secondProvider = NotifierProvider<SecondsNotifer, int>(() { | ||
| return SecondsNotifer(); | ||
| }); |
There was a problem hiding this comment.
Fix typo in class names
There's a typo in the class names: MinutesNotifer and SecondsNotifer should be MinutesNotifier and SecondsNotifier (missing 'i' in "Notifier").
-final minutesProvider = NotifierProvider<MinutesNotifer, int>(() {
- return MinutesNotifer();
+final minutesProvider = NotifierProvider<MinutesNotifier, int>(() {
+ return MinutesNotifier();
});
-final secondProvider = NotifierProvider<SecondsNotifer, int>(() {
- return SecondsNotifer();
+final secondProvider = NotifierProvider<SecondsNotifier, int>(() {
+ return SecondsNotifier();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final minutesProvider = NotifierProvider<MinutesNotifer, int>(() { | |
| return MinutesNotifer(); | |
| }); | |
| final secondProvider = NotifierProvider<SecondsNotifer, int>(() { | |
| return SecondsNotifer(); | |
| }); | |
| final minutesProvider = NotifierProvider<MinutesNotifier, int>(() { | |
| return MinutesNotifier(); | |
| }); | |
| final secondProvider = NotifierProvider<SecondsNotifier, int>(() { | |
| return SecondsNotifier(); | |
| }); |
| class SecondsNotifer extends Notifier<int> { | ||
| @override | ||
| int build() { | ||
| return 0; | ||
| } | ||
|
|
||
| void increment() { | ||
| var nextState = state; | ||
| nextState = nextState + 1; | ||
| if (nextState == 60) { | ||
| nextState = 0; | ||
| } | ||
| state = nextState; | ||
| } | ||
|
|
||
| int value() { | ||
| return state; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary value() method and simplify increment logic
- The
value()method is redundant as the state can be accessed directly through the provider. - The increment logic can be simplified similar to the minutes notifier.
class SecondsNotifer extends Notifier<int> {
@override
int build() {
return 0;
}
void increment() {
- var nextState = state;
- nextState = nextState + 1;
- if (nextState == 60) {
- nextState = 0;
- }
- state = nextState;
+ state = (state + 1) % 60;
}
- int value() {
- return state;
- }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class SecondsNotifer extends Notifier<int> { | |
| @override | |
| int build() { | |
| return 0; | |
| } | |
| void increment() { | |
| var nextState = state; | |
| nextState = nextState + 1; | |
| if (nextState == 60) { | |
| nextState = 0; | |
| } | |
| state = nextState; | |
| } | |
| int value() { | |
| return state; | |
| } | |
| } | |
| class SecondsNotifer extends Notifier<int> { | |
| @override | |
| int build() { | |
| return 0; | |
| } | |
| void increment() { | |
| state = (state + 1) % 60; | |
| } | |
| } |
|
Pure code without any explanation isn't a good way to explain what happens. Also, there's already a page dedicated to that: https://riverpod.dev/docs/essentials/combining_requests |
|
I think this example is really simple, easy to run in a simple copy paste, and you might be able to extend it by incrementing the second provider automatically at a specific interval. I think it's much accessible than the current example in the documentation you pointed. I think you should consider it in the current documentation. |
|
As I said, that's too "raw". It's just code without any explanation |
|
My question is, with explanation, what's your thoughts on this example ? Do you consider to add it, and if so, where? |
|
Depends on a lot of things. If it's about having a simpler example, that example is likely too big. A few lines at most should be the goal |
|
"Combining requests" is such a weird title because Riverpod doesn't do any requests. You have a notifier listening another notifier, that's what you want to achieve with Riverpod, not combining requests. |
That's literally the hello word of listening to another provider with an UI to be able to test it, how would it be shorter? |
Most of people read the doc, not the example directly |
|
I submitted it, because I wish I found this example when learning and doing my test with Riverpod Something simple to just run and put breakpoint to learn a basic way of using Riverpod |
|
If it's something that folks should be able to run, that shouldn't be as a document. That's in the example folder. If it's just a snippet to showcase a feature, that doesn't need to be executable. It just needs to showcase the feature. In which case we don't care about widgets & stuff here |
do you agree? or do I miss something? what are requests in Riverpod?
do you want to do something with this example, if so, what? or do you want to drop it? from what you describe you will put it in the example folder? |
Network requests. Riverpod's primary goal is network requests. Although the name isn't super clear. We could likely rename it to "How to make a network request that depends on another one". It feels like you're asking for a list of "concepts" in the docs (which the current site lacks, as it's mainly focused tutorials). |
|
From my understanding, @rrousselGit Riverpod doesn't make any Network requests. It's not a Network library. |
|
Riverpod's primary goal is network requests. That's not really a debate :) It doesn't mean we can't have general-purpose docs. But network requests still are the primary focus |
|
Riverpod doesn’t directly deal with network requests—it only interacts with values or futures. While network requests might be the primary use case for many, it’s surprising that the broader applications of what you’ve built aren’t fully acknowledged. For instance, if the data for an app resides on local disk instead of a server, the fundamental needs remain the same: invalidating state, supporting pull-to-refresh, and so on. Riverpod would handle these scenarios just as effectively, without requiring any additional packages. It’s similar to building a scheduler but saying it’s focused solely on Linux processes. A scheduler is inherently platform-agnostic—it isn’t tightly coupled with Linux, nor should it be. Likewise, Riverpod is not a network library, and it’s important to recognize its potential across a wide range of use cases beyond just network request |
|
I think you're conflating a few things here. And to counter your point, it's fairly likely that at some point in the future, Riverpod will offer a way to do network-requests natively. I've considered many times shipping a custom HTTP client, or some form of HTTP-aware Repository |
|
What advantages would it bring to tightly couple Riverpod to an HTTP client rather than letting the user choose its usage or implementation? Can you name any? Can you name anything that Riverpod can do that is network-specific and doesn't make sense for other types of resources? Can we keep this talk on the current state of Riverpod rather than an imaginary future as an argument on what Riverod is? Keeping the HTTP client out of Riverpod made you solve the general problem rather than the specific one, I’m really surprised it was not intentional. Why did you keep the HTTP client out in the first place if your issue was network requests? |
|
While I can answer those questions, we're getting fairly off topic here. The state of the PR stays the same: |
|
@rrousselGit, my comment here isn’t about the PR itself but rather about the documentation. Here’s my perspective:
This leads me to the following questions:
If my understanding is incorrect, I’m eager to learn why. And if these questions reveal gaps in the current perspective, perhaps there’s an opportunity for mutual growth. |
|
I don't see how those questions move us any closer to resolving this PR. The answer to those questions shouldn't matter for a doc about combining providers. |
|
@rrousselGit, you’ve avoided addressing my straightforward and technical questions twice now. These aren’t about your "vision" but rather the technical design decisions behind Riverpod. I’m not sure why you’re hesitant to engage in this discussion. If you’re unwilling to explore why I think network requests are a special case of what Riverpod solves, I’ll leave it at that. That said, I believe these questions are worth reflecting on. They could help refine Riverpod's goals and purpose, or even reveal broader use cases beyond those currently emphasized. I hope you take a moment to consider them—it might click for you. |
|
You're just arguing alone and in the wrong place here. We should be working toward how to merge or close this PR. If you want to debate why Riverpod's docs talk about network requests so much, that's fine. But that's not the right place to do so. Either Discord or a dedicated issue would be the right place. Here, we should only be talking about topic relevant to your changes |
|
I’ve created an issue for this discussion, I hope you can teach me something there |
|
Regarding the pull request Also, concept is quite a good place to get started with Riverpod but there is no link to it when you start in “Choose your next step” what is your thoughts on that? |
|
The combining request page is specifically about HTTP. At least it should be. So I don't think that's quite the right place You want something logic agnostic, so a concept folder makes sense Feel free to add crosslinks to new docs wherever you think it makes sense |
|
Closing. While I appreciate, I don't think I can accept this as it is. If you still want to contribute something here, do you mind making an issue about it so that we can discuss what such contribution would look like? |
|
@rrousselGit should we just add it to the riverpod example folder? |
documentation improvement by adding a simple example for beginner
Summary by CodeRabbit