rrousselGit / riverpod

A reactive caching and data-binding framework. Riverpod makes working with asynchronous code a breeze.
https://riverpod.dev
MIT License
6.26k stars 959 forks source link

Document some tips on how to maintain a project using Riverpod (aka use granular reactivity APIs efficiently) #3002

Closed lucavenir closed 7 months ago

lucavenir commented 1 year ago

The combination of these two pages (1, 2) isn't enough for folks (at least - the ones I've been in contact with), yet. I think we need another paragrah or article cross referencing these two, bringing more clarity about the following.

Describe what scenario you think is uncovered by the existing examples/articles

One thing I think we've missed is that pkg:riverpod differs from pkg:provider in these two important aspects, before anything else:

Describe why existing examples/articles do not cover this case

Folks often look forwards to work with pkg:riverpod because it's either seen as a "better provider", or they start anew with it just because it's popular. But then, they often collide with it. The most asked question I get is:

how can I migrate my centralized state?

Folks often write code that looks like this:

class StateManagementExample extends ChangeNotifier {
  // prop with getter / setter attributes
  final List<Todo> todos = [];
  // computed prop, but here its computation is often handled imperatively
  final List<Todo> filteredTodos = [];
  // computed prop, but it's just an handy getter
  int get amount => filteredTodos.length;
  // more props ment to be set / read
  bool canAddMoreTodos = true;

  // TODO write side effects, mutations, `notifiyListeners()`, etc.
}

First - and let me underline it - this is very common. I've seen the above very often. Sure, migration the above really depends on folks' use case, but there must be some sort of guidance for it. Still, there's a lot of space for showing off pkg:riverpod capabilities, such as a better and more explicit separation between mutable state and computable (aka reactive) getters.

Second, folks are confused as they find out that there's no direct and equivalent state management class in riverpod. Sure, we find it trivial, but they get - rightfully - confused and ask questions like:

  1. Why can't I have one class encapsulating all the state I need for this widget / page / feature?
  2. Why must write a lot more code just to make my state "work"? Does writing mutiple providers bring any kind of advantage, at all?
  3. ChangeNotifier allows me to write a facade-like API, how can I do that for riverpod? I can I write future-proof and maintainable code with riverpod?

Sure, we do know there are concrete advantages and differences, but folks don't, and I don't think they can see (or read 😄) that yet.

I'm opening this issue as I'm trying to collect some "good arguments" that can be put in a ad-hoc paragraph about this, e.g.:

Additional context

While it's important not to show to-be-avoided practices, it's also important to show guidance / to offer a concrete solution to the above problem.

In a nutshell, this issue could be summarized as: folks don't understand why centralizing logic is discouraged in Riverpod. How can we see "the way" without using notable anti-patterns and preserving lints?

rrousselGit commented 1 year ago

I wouldn't focus too much on migration content.

It's nice, but there's more important things.

lucavenir commented 1 year ago

Please, give it another shot by changing the POV; take this issue as an "enhancement proposal". Why isn't the following a thing, with riverpod?

@riverpod
class MyState extends RiverpodState {
  @state
  Future<List<int>> todos(TodosRef ref) {
    // classic `AsyncNotifier` build logic
  }

  @state
  bool canAddMore(CanAddMoreRef ref) {
    // independent, non reactive state, which gets initialized as `true`
    // but it has some degree of kinship with the above controller
  }

  Future<void> add(Todo todo) {
    if (canAddMore)  // side effect which is meant for `todos`
    if (someCondition) canAddMore = false;
  }
}

Which - allegedly - would allow encapsulation, centralization and more "open-closeness" of our codebases? See usage:

// watches the whole facade
final wholeState = ref.watch(myState);
// watches just my todos
final computation = ref.watch(myState.select((state) => state.todos));
// adds some todos
ref.read(myState.notifier).add(...);

The main argument used out there is that global scoped providers can be used "everywhere" and they bring "confusion": you don't know "where to look at" or "how to minimize time wasted searching for the granular provider around the codebase".

rrousselGit commented 1 year ago

I don't think that's a common question

A centralized state isn't something pushed by Provider either. It also pushes granular states.

lucavenir commented 1 year ago

Okay, one last try and I won't bother you anymore 😄

That's good to know I have some bias, but... are you telling me you've never received a critique like...

Riverpod doesn't scale as well as Provider/BLoC/etc.

Because that's the source of all the discussions that bring me down this path. People want to minimize DX collision in large teams, so that even new devs can easily understand a riverpod-based project, instead of having a hard time finding... stuff.

Thankfully, software architecture helps, but up to some extent. I'll make up a final example that tries to sum up a situation that has been brought to my table from a colleague, an - allegedly 😄 - experienced dev:

A: Hey, I'm working on the Todo feature, but I can't seem to find the filtered ones B: OH yeah that's in another provider,filtered.dart A: Rrriiight, and where can I look at the filters themselves? B: Sure, it's in a separate controller,filters.dart. Luckily we're talking about the same feature, so you'll find everything you need in that directory! A: Ugh, fine. Listen, our client now requested a requirements change, it looks that filters now should take into account a new server-side response before being applied. Where do I put this new logic into? B: Oh, well, just create a new FutureProvider, and then let the filters depend on it A: ... Damn, now our filter is async, thus everything changes in cascade [proceeds submitting a 400 lines diff because we're dealing with AsyncValue now]

Whereas, the same identical dev used to quickly solve the above discussion with pkg:provider, like so:

A: Hey, I'm working on the Todo feature, but I can't seem to find the filtered on--- oh wait, it's right there, in the same ChangeNotifier instance B: Yeah. We don't need no FiltersNotifier, as our ChangeNotifier contains everything we need. A: Yeah I love that writing a dot is enough for my IDE to give me everything I need within this context. Okay, I'll work on the new requirements now [proceeds writing a few more LoC without ever touching previously written working code] B: Life's good 🍺

Okay, jokes aside, I think you get the idea. Of course there's my colleague's bias, and my inability to follow SOLID principles using Riverpod, but I can't see how to solve these problems reliably while working with riverpod in big teams.

Maybe I should change this issue's topic: "Add a "Scaling up" section". I'm not sure. I'm kind-of disheartened right now. Please show me a path here 😆

rrousselGit commented 1 year ago

The kind is discussion you're referring to is not something I've seen come up, no.

To be honest, I've seen very few people ask me how to migrate from Provider to Riverpod. On the other hand, lots of people asked about how to do pull-to-refresh or infinite lists.

The issue about pull to refresh had tons of :+1: for instance.

Whereas, the same experience dev, used to quickly solve the above discussion with pkg:provider.

How so? It's litteraly the same issue. you need to find where the provider is defined I'd you want to edit it. But hey, rather than being in a separate file, you now have to look in a random UI file to find it. Worse, there may be multiple providers for the same value! Tests often redefine a new provider for example.

And where your filter is? filter.dart too.

Riverpod gives you autocompletion. That's much better than what provider offers. There's "find all references" too. Heck Riverpod even has a graph generator which you can run on your project.

And the devtool is coming anyway, so whatever is in the app could be found there too. We could easily show the file path / provider name in here.


If someone puts everything in a single ChangeNotifier, I see no reason why they wouldn't know how to do the same in Riverpod.

They should already know how to use ref.notifyListebers. From there they can make a state as big as they wish.

lucavenir commented 1 year ago

Thank you, I think I'll close this, but..

How so? It's litteraly the same issue. ... And where your filter is? filter.dart too.

I did not understand this whole paragraph.

They should already know how to use ref.notifyListeners. From there they can make a state as big as they wish.

So, the trade-off is: do you want it centralized? Use ChangeNotifier. Do you want it fine-grained? There's the other APIs.

rrousselGit commented 1 year ago

I did not understand this whole paragraph.

I'm just saying that what you're describing is not a property of Riverpod vs Provider. And that Provider suffers from the same issues

You're talking about centralized VS granular state. Both Riverpod, Provider and Bloc fall in the category of granular state. Folks using a centralized state with either of these approaches are not using the packages as they are intended to be.

That discussion could make sense in the context of Riverpod vs Redux. But not Riverpod vs Provider

lucavenir commented 1 year ago

Both Riverpod, Provider and Bloc fall in the category of granular state. Folks using a centralized state with either of these approaches are not using the packages as they are intended to be.

That's news to me, especially for Provider/BLoC. But that's perfect, because at least this answers my doubts.

As a side note (just to show my own bias), everyone I've worked with up until now (Flutter-wise) chose BLoC / Provider over Riverpod mainly because of the availability of a "centralized approach", alone. The common sentiment in my circles is that it's harder to maintain a project using riverpod since it kind-of enforces granularity, and that might hurt in the long run.

All in all, maybe a mention on how to maintain a "granular-state"-based project could be useful.

I'll leave this open with a new title; close it yourself if you think such title shouldn't appear in the docs.

rrousselGit commented 1 year ago

That's certainly an odd view.
Most docs regarding Provider or Bloc tend to have at least one "controller" per view.

Having a big "app state" object is certainly not something I've seen promoted with those packages. I wonder where you saw that from.

In any case, migration content overall is always going to be quite niche. Showcasing how to use the package reaches a larger audience. It's helpful to everyone, including those who want to migrate from another approach.
Hence, I personally won't focus on such content and wouldn't recommend picking such topic for a PR either.

lucavenir commented 1 year ago

Gotcha. Even if this wouldn't be about migrations anymore, I see your vision.

Final remark:

Having a big "app state"

That's not what I meant. Imagine one "less big feature state" object that self-contains anything regarding the use cases of that feature. Which on paper is also clean-architecture compliant.

rrousselGit commented 1 year ago

General organizational practices could be valuable, yes.

There's a DO/DON't page with a limited number of points https://riverpod.dev/docs/essentials/do_dont#avoid-dynamically-creating-providers We could add many more suggestions in there.