Skip to content
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

Sync Providers managing the same entity #3781

Open
w0rsti opened this issue Oct 16, 2024 · 46 comments
Open

Sync Providers managing the same entity #3781

w0rsti opened this issue Oct 16, 2024 · 46 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@w0rsti
Copy link

w0rsti commented Oct 16, 2024

Describe what scenario you think is uncovered by the existing examples/articles
It is quite common to have a List provider as well as an Item provider, especially when dealing with
web applications and (deep) links. It is not well documented on how to share state between provider that might
contain the same Item. To be more precise on what I mean, lets take a look at an example:

Lets suppose we develop an Web-Application with the following paths:

https://my-awesome-todo.app/todos/
https://my-awesome-todo.app/todos/:id

and a simple model of the todo as the following:

@freezed
class Todo with _$Todo {
  const factory Todo({
    required String id,
    required String title,
    required String description,
    required bool isCompleted,
  }) = _Todo;

  factory Todo.fromJson(Map<String, dynamic> json) => _$TodoFromJson(json);
}

As you can see, it is a pretty simple setup: /todos shows us an overview of the todos (e.g. only the title and the completion status) while /todos/:id/ will display an detailed version of that todo (e.g. title, status, description).
Since this is an (web-) application, the user might navigate to the detailed screen without ever rendering the overview screen.

Lets start pretty simple - just have two separate providers that fetch the todos/todo:

Future<List<Todo>> fetchTodos() async {
  // Some async logic that fetches the todos
  // e.g. from a remote database.
}

Future<List<Todo>> fetchTodo(String id) async {
  // Some async logic that fetches the todo
  // e.g. from a remote database.
}

@riverpod
Future<List<Todo>> todos(TodosRef ref) {
  return fetchTodos();
}

@riverpod
Future<Todo> todo(TodoRef ref, String id) {
  return fetchTodo(id);
}

Okay - fair! But now the first problem raises:
We might fetch the same entity twice - first when the user visits the overview and second when he visits the detailed view - pretty wasteful on ressources, no? You might argue now that this can be fixed fairly easy by awaiting the todos and returning the todo as a dependent provider.

@riverpod
Future<Todo> todo(TodoRef ref, String id) async {
  final  todos = await ref.watch(todosProvider.future);
  return todos.where((todo) => todo.id == id).first;
}

But now we still fetch the whole list just to get that one provider, so we can even optimize it further...

@riverpod
Future<Todo> todo(TodoRef ref, String id) async {
  // Check if the provider exists, so we don't initialize it
  if(ref.exists(todosProvider)) {
     // Just check the docs on ref.exists, it describes this.
     final todo = ref.watch(todosProvider).selectAsync(/* ... */);
     if(todo != null) return todo;
  }

  return fetchTodo(id);
}

Okay. Understandable to this point - TLDR; The detailed provider should check the list provider if it exists, and if yes, check its state so we don't fetch an todo twice - seems logical!

But now lets suppose we don't have a single todosProvider, this can be the due to multiple reasons:

  • Reason 1: The Todos Provider doesn't fetch all the todos, only those that are not completed (=> isCompleted: false). And we have another Todos Provider (uncompletedTodosProvider) that fetches the uncompleted todos. Now we need to modify our todo provider to check 2 possible lists...
    In this case is a little trivial but assume we have something else with a lot more properties and a more complex data model, we could end up with 3-4 places to check. While adding all of those providers to check first before fetching the actual item seem a little annoying, I am willing to accept it BUT:

  • Reason 2 (prob. more common): The Todos Provider is actually a FamilyProvider since we have several thousand of todos (overall we are pretty busy, no?) so that the fetching of all todos would take a long time and a lot of ressources. So we simply add a int page
    to the params of the todosProvider - Lets make a loop to check every familyProvider(page)... but wait - how far should we check? We don't know how many pages there might be...

Let's mentally reset and assume we found a solution OR just go with the simple overfetching - we fetch our todo it in the todosProvider aswell as in the todoProvider 🤷🏼‍♂️

But aren't Todos supposed to be updatable? I mean we somehow want to check of our todos, right? This means we have to make our todoProvider an class based (notifier) provider.. But updating the state of this provider doesn't update it within the overviewProvider and vice versa.. How can we keep them synchronized?

There just doesn't seem to be an sufficient example that is "complex" enough to showcase how to use riverpod with
this common use-cases above.

I hope with this little example made my problem clear - if not, I am happy to discuss your solutions and will throw further constraints at you! :D

Describe why existing examples/articles do not cover this case
The docs on the riverpod website cover just simple use-cases, either
using read-only (family) provider or local/simple state notifier.
I have been struggling with the problem mentioned for a long time now
and can't find a general solution to tackle this problem. It seems like
other people having problems with this problem (or a related one) too.
E.g. #3285

@w0rsti w0rsti added documentation Improvements or additions to documentation needs triage labels Oct 16, 2024
@rrousselGit
Copy link
Owner

This is a complex problem, and folks have raised issues asking how to deal with his before.
There's no finite solution here, and some 3.0 features make this simpler.

I'll keep this open as a reminder to document this.

@w0rsti
Copy link
Author

w0rsti commented Oct 17, 2024

Can you elaborate which features Riverpod 3 will offer that makes this simpler?

Is there any ETA on Riverpod 3.0?

@snapsl
Copy link

snapsl commented Oct 18, 2024

I would recommend to adapt a programming pattern like the repository pattern. The repository implements your business logic including caching, pagination, filtering, etc... . Then you can build your state management using riverpod around this repository.
The result is a clean separation of concerns which avoid these common pitfalls that you described. Many great articles demonstrate how to implement this.

@w0rsti
Copy link
Author

w0rsti commented Oct 18, 2024

@snapsl The repository pattern is mostly referred to as an wrapper around multiple data sources and is used to abstract away which data source is being used and how data is being fetched/modified. As far as I understand, a repository is stateless and has "nothing" to do with state except it's the gate to the outside world aka. the data layer.

I don't quite see how this is solving the issue mentioned above, happy to hear about how you will solve it.

I've read a lot of articles about the usage of riverpod but either they don't run into this issue since they are to simple or they don't don't adhere to the intended way on how to use riverpod. (E.g. Andreas Popular Post about riverpod architecture - I've seen a lot of comments by remi that this and that is not how riverpod is intended to be used).

But as said, I am happy about further information or your proposal on how to tackle the above mentioned issue.

@snapsl
Copy link

snapsl commented Oct 18, 2024

Let's stay with the todo example and create a small sample todo repo to make this more clear.

class TodoRepo {
  
  TodoRepo(LocalSource, RemoteSource, ...);
  
  List<Todo> getTodoList()...;
  Todo getTodo(String id)...;
  Todo updateTodo(String id)...;
  ...
}

This holds all the implementation regarding todos (very simplified). A single source of truth that is encapsulated, mockable, and testable. Cool!

Your state management then uses this todo repo.
Thus we need a service locator for it e.g.

@riverpod
TodoRepo todoRepo(Ref ...) => TodoRepo(....);

and use it for our state management.

@riverpod
class Todos extends _$Todos {

  List<Todo> build() => ref.watch(todoRepoProvider).getTodoList();
  
  void updateTodo(String id) { 
    final todo = ref.read(todoRepoProvider).updateTodo(id);
    state = ...;
    ref.invalidate(todoProvider(id));
  }

 ...
}

@riverpod
Todo todo(Ref ref, String id) => ref.watch(todoRepoProvider).getTodo(id);

Fixed:

  • Redundant fetches: The repository either has the data locally or not.
  • Sync todos: This is not specific to the repo but you need a 'main' provider that manages the other provider e.g. invalidate.

In general there is a reason for these programming patterns to exist since other devs had the same problems and came up with solutions that we should use to not make the same mistakes.

I hope this helps you with the real application :)

@rrousselGit
Copy link
Owner

Making a repository is only a workaround in Riverpod's case.
You should have a more official solution to the problem.

I won't go into detail here as things are still WIP. But I do want to improve this.

@snapsl
Copy link

snapsl commented Oct 18, 2024

@rrousselGit creating a riverpod pattern 👍

@w0rsti
Copy link
Author

w0rsti commented Oct 18, 2024

@snapsl
First of all, thanks a lot for joining and staying within this discussion and your feedback/proposals!

I think there are multiple scenarios why this might not work:

Reason 1:
The list is paginated, while the editing operation is performed in a detail view.
Imagine you show an overview page of the todos. As said, we are pretty busy so we might have thousands of todos and therefor instead of fetching all the todos, we have a paginated API which results in a "paginated" provider, so let me modify your examples/functions a bit:

class TodoRepo {
   ...
  // Fetches the given page of the todos, each containing e.g. 20 todos 
  List<Todo> getTodoList(int page)...;
  ...
}
@riverpod
class Todos extends _$Todos {

  List<Todo> build(int page) => ref.watch(todoRepoProvider).getTodoList(page);
  
  void updateTodo(String id) {
      ...
   }
 ...
}

As mentioned in the original issue, lets still assume we are working on a web application where the user might directly navigate to https://my-awesome-todo.app/todos/:id. There is no way that we can determine the correct (paginated) list provider to call the update method on - since we don't know in which page the todo is. As a matter of fact, the todos provider is not even alive/initialized.

Reason 2:
For some reasons, an todo might end up in multiple lists. For this lets modify the Todo Model a bit:

@freezed
class Todo with _$Todo {
  const factory Todo({
    required String id,
    required String title,
    required String description,
    
    /// The date and time the todo is scheduled to be
    /// worked on/completed.
    /// If this is null, the todo has not been scheduled yet.
    DateTime? scheduledAt,
    
    /// The date and time the todo has been completed.
    /// If this is null, the todo has not been completed yet.
    DateTime? completedAt,

    /// The date and time the todo has been completed.
    /// If this is null, the todo has not been completed yet.
    required DateTime createdAt,
  }) = _Todo;

  factory Todo.fromJson(Map<String, dynamic> json) => _$TodoFromJson(json);
}

As you can see, the bool isCompleted has been replaced by a timestamp and some properties (scheduledAt, createdAt) have been added - just some more data so I can create a realistic scenario.

Now lets assume we have some widget/page in our app that displays:

  • Recently created Todos
  • Recently completed Todos
  • Scheduled Todos

For each of these we can make an API call that returns a List:

  • Recently created Todos -> API Call returns the last 10 created todos based on "createdAt"
  • Recently completed Todos -> API Call returns the last 10 todos that have been completed
  • Scheduled Todos -> API Call returns all scheduled todos

For each of those, we have a provider of course... I don't think I have to show an example on how to create them.

Naturally a todo might end up in multiple lists:
Assume I just created an todo and I scheduled it for tomorrow - It might end up in the "Recently created Todos" Provider as well as in the "Scheduled Todos" Provider.

Where should I place the update method? In every provider?
You could argue that every ListProvider should expose an updateTodo() function that updates their internal state,
but who is responsible for the api call?
We could wrap the call in an additional Service (or call it whatever you want) class that is responsible for the api call and updates all the providers, but this feels really imperative. Now when creating a new list provider for some sort, I need to remember to refactor the "updateTodo" to also call that new list provider.

This is what #3285 was initially about as far as I remember (the issue has been edited)

I hope I gave you some understandable and "real-life" why I think your proposal does not solve the issue.
I am happy to hear back if you have a fix/solution for these scenarios or more thoughts!

@snapsl
Copy link

snapsl commented Oct 18, 2024

Okay, I am not sure if it helps to stay with the todo example. But this is still the same principal.

  1. Add these functions to the repository.
class TodoRepo {
  List<Todo> get listTodo = []; 

  List<Todo> getPage(int page)...;
  
  List<Todo> getRecentTodos();

  ...
}
  1. Build your state management around this repository.

  2. The 'main' todosProvider implements crud operations and updates / invalidates dependant providers.


Note:

  • Assuming RecentTodos are implemented as some sort of streaming / websocket. This should probably not be locally stored and always be refreshed.
  • Filtering (e.g. Datetime, completed todos): If you can't store all the todos locally filter operations need to be done in the backend.
  • Pagination: Don't make the todosProvider a family provider. You need this for your crud operations. Handle the pagination in a separate provider or abstract it away using the repository.

@snapsl
Copy link

snapsl commented Oct 18, 2024

As mentioned in the original issue, lets still assume we are working on a web application where the user might directly navigate to https://my-awesome-todo.app/todos/:id. There is no way that we can determine the correct (paginated) list provider to call the update method on - since we don't know in which page the todo is. As a matter of fact, the todos provider is not even alive/initialized.

See above

@riverpod
Todo todo(Ref ref, String id) => ref.watch(todoRepoProvider).getTodo(id);

@w0rsti
Copy link
Author

w0rsti commented Oct 19, 2024

@snapsl
I think I kind of getting the direction this is going just some questions:

  • What would be the initial state then of the todosProvider (the "main" one) - always an empty list?
  • Since I invalidate the providers from the "main" todos provider - the todos provider needs to know about every provider that uses the repo... isn't there a more reactive way?

In order to keep this thread cleaner, I am also down to discuss further on discord if that's okay with you: My name there is same as in Github w0rsti

@lucavenir
Copy link
Contributor

Hello! 😄

It's been a while since I've tried to tackle this problem, and since then I've recently updated the original issue with a summary of the problem:

An app shows a list of Books in its home page, but also allows me to search for a Book in a search page. Then again, each book has its detail page. Finally, there's a Library page, showing different libraries, each with their list view of Books. Every list mentioned above is paginated.

Here's the twist: every book is likeable, aka all entities here share this common property.
How can I properly keep track of a book's bool isFavorite property, in every different page?
A typical user flow is, e.g., a book is liked in the library page; pressing "back", the home shows this book's favorite state updated accordingly.

Here's an additional twist, regarding our source of truth: While the user navigates the app, state changes might occur in other places as the navigation occurs (e.g. in a separate web app). So to optimize for our user's experience we need to implement a "WYSIWYG" approach, in which, the latest state received from our server is "the truth".

I want to share my 2 cents and my experience about this.

First, it's best to recognize that this is quite a hard problem. We're exploring boundaries that most client-side applications get wrong, even popular ones.

I, too, used a "repository pattern" to solve this in the past months, and I'd definitively advise against such an approach. Assuming you're suggesting this repository should save on a local cache whenever new data is received from our network (e.g. with sqlite):

  1. Clearly, it's just a workaround, and you'd have the feeling of designing something flawed - and indeed we're writing a repo just so we can build our state management around it, that's a smell, clear as day
  2. We're re-implementing what Riverpod is supposed to do (the caching part especially, considering Riverpod is on its way to support offline mode)
  3. We're adding another layer of indirection, which is usually useless, but it's even harmful in this case: we cannot really afford to hide away the decision of hitting the local db VS hitting a network request, because (again) this eventually collides with what Riverpod tries to do (generally speaking, I'd advise dropping the repository pattern when working with Riverpod)
  4. Following this path is totally doable, but in my experience it's quite easy to mess it up; e.g. it's quite easy to hit the network "too much" (i.e. fill our server with useless requests)

I want to share another workaround while we wait for R3' docs.

Given we have some "shared state" family providers, e.g.: homeItems(int page), searchItems(int page, String query), itemDetails(int bookId), favoriteItems, etc., we could define a FavoritesOverrideController exposing a Map<int, bool>, initially empty.

The idea is to save "divergent data" on a separate hash-like data structure (here I'm just saving the bool isFavorite, but you get the idea).

In my use case, whenever we hit a "like" button, we write an override in there, meaning: "this has been touched, and the source of truth has now changed".
To synchronize state and to have a fresh cache, whenever we hit the network, we overwrite the overrides, if any, with fresh state.
There's a few flaws to this approach (e.g. if a user gives infinite likes to each and every item he encounters, we'd eventually reach an oom error), but it works.

If you're curious I'm working on a repository on this topic since a while, which I've just updated. You can check out my implementation example, there.

I can't wait for Remi's solution and for R3!

@w0rsti
Copy link
Author

w0rsti commented Oct 22, 2024

@lucavenir Thanks for joining this discussion!

I also tried a lot of different solutions that I've tried, e.g. caching and streaming from a repository class ("watchXY -> Stream" and every update emits a the updated value...), listen to the "single item" provider and creating it with already fetched values , by creating a class for the params e.g. "ItemReference" where there is a mandatory id property and an optional value property and overwriting the hashCode and equality operator -> This was you can pass "initial" data to a provider, since an "ItemReference" with only id is equal to an ItemReference with a value.

The only way that doesn't feel like actually working AGAINST Riverpod came to my mind after reading your comment, so I still have to try how it feels:

Create an "ItemSyncService" - A (family) Notifier which state represents the "cached" value.

  • It should have a set/update (whatever you wanna call it) method, so every provider can "set" the cache.
  • Every provider interested in using the latest Item, can listen to it and update its own state accordingly.
  • Every provider that does some mutation to that item should update the corresponding SyncService, so other providers can react to it.
  • This should take into account nullable values (e.g. if the item does not exist) as well as a way to support optimistic updates, which can be done by a simple state (either as tupel/typedef or actual class):
typedef ItemSyncServiceState = ({
/// The currently cached value, this is nullable since initially we have no value OR in case the item does not exists
Item? value,

/// Used to determine if some provider set the state or if we haven't fetched it yet.
bool hasValue,

/// Used for optimistic updates. When some provider updates this state optimistically, the pending future should be
// set here so other providers can await that future and revert to their "old state" when the operation fails. 
Future? pendingOperation,
});

This way if no one is interested in updates to that item or no one is modifying it, the provider will get disposed.
We can also keep this provider alive for x amount of time to keep the value available as cache for x amount of time.

Would appreciate your opinion on it and if you thought about it already too or even tried it.

@elkSal
Copy link

elkSal commented Oct 27, 2024

I also was thinking about this problem as well.
Let's assume I have a method in the repository to stream a batch of 50 todos and if user requires to stream +50 todos on top of the ones already available.

final watchTodosProvider = StreamProvider.family(
         (ref, params) => return ref.read(todosRepositoryProvider)
             .streamTodos(from: params.from, to: params.to)
 );

This way I have N streamproviders watchTodosProvider, each one with 50 todos.
Is there a way to merge these N watchTodosProvider into a single streamprovider?

If I stream the first 50 and then stream 50 more, I could change the params.to parameter but this solution is not effective as:

  • it would trigger refetching all the 100 todos, without using the 50 todos already cached
  • keeping the N streamProviders separated allows less rebuilds as if one todo changes it would affect only the batch it is in

@Madnex
Copy link

Madnex commented Nov 4, 2024

Hi! I would like to second the importance of this problem.

I'm relatively new with flutter and have started the first more complex app project using provider.
After realizing I'm in the same situation described here I searched quite a bit for a solution and was surprised to not find much about this topic specifically (maybe because it's not easy to describe -> no clear idea what to search for 😅).

I was thinking about following Andrea's architecture but after reading through this issue here I also don't see how this necessarily solves the problem at hand.

It would be amazing if riverpod would make this easier / possible and documentation about this would be unprecedented from what I could find 👀

On the risk of diverging from the specific riverpod solution: One thing that I also found regarding this topic was the offline first approach. But that sounds too good to be true to me? Local db as state manager? If the app use case allows to store the entire (or required selected) data in a local db would that solve the problem of outdated data in different places?

@RockStone
Copy link

Can you elaborate which features Riverpod 3 will offer that makes this simpler?

Is there any ETA on Riverpod 3.0?

same. 3.0-dev has been stalled for almost a year. is it still WIP?

@lucavenir
Copy link
Contributor

same. 3.0-dev has been stalled for almost a year. is it still WIP?

  1. This is an open source platform, not a project management board
  2. AFAIK The maintainer chose not to publish any updates to avoid public leaking of not-yet-stable APIs; also AFAIK riverpod v3 will be released with the documentation, therefore no dev previews will be released in the meantime

@rrousselGit
Copy link
Owner

FWIW I'm currently investigating a sort of "bundle of provider".

As in, a Repository-like with multiple source of states:

@someAnnotation
class Users extends _$Users {
  @riverpod
  Future<User?> byId(String id) => /* http.get(...) */;

  @riverpod
  Future<List<User>> home() => /* http.get(...) */;  
}

Where Riverpod would automatically synchronise all User instances based on User.id

@rrousselGit
Copy link
Owner

rrousselGit commented Dec 5, 2024

My main challenge at the moment is figuring out how to enable folks to do a list.add and other mutations.

I kind of wish Dart had nested classes. We could then do:

@riverpod
class Users {
  @riverpod
  static Future<User?> byId(String id) => /* http.get(...) */;

  @riverpod
  class Home {
    @override
    Future<List<User>> build() => /* http.get */;

    Future<void> add(User user) => state = [...state, user];
  }
}
...

AsyncValue<List<User>> users = ref.watch(usersProvider.home);
Home home = ref.watch(usersProvider.home.notifier);

@snapsl
Copy link

snapsl commented Dec 5, 2024

That is great progress! 👍
Thank you @rrousselGit for keeping us up to date.


What about the mutation syntax introduced in #1660?
The return value of the mutation would be the updated state.

@mutation
Future<void> add(User user) {
   return [...state, user];
}

A kind of reactive “view” (I couldn't come up with a better name) managed by the same entity would solve this problem.
What do you think about following syntax that would not require nested classes?
Whenever a mutation is invoked each reactive "view" would update its state.
Thinking about it, the mutation method should update the individual "view" state:

@riverpod
class Users {
  @reactive
  Future<User?> byId(String id) => /* http.get(...) */;

  @reactive
  Future<List<User>> home() => /* http.get(...) */;

  @mutation
  Future<void> addUser(User user) => /* ... */;    
}

AsyncValue<List<User>> users = ref.watch(usersProvider.home);

AddUserMutation mut = ref.watch(usersProvider.addMutation);

@rrousselGit
Copy link
Owner

Defining a method isn't the issue. I already have working mutations.

The problem is about how to do things like state = state +1. You need the previous state to compute the new state.
With normal notifiers, there's this.state/this.future. But they don't quite apply here

@snapsl
Copy link

snapsl commented Dec 5, 2024

Each view would have its own state that needs to be updated. E.g.

// home view 
@reactive
Future<List<User>> home() => /* http.get(...) */;

// recent view 
@reactive
Future<List<User>> recent() => /* http.get(...) */;

@mutation
Future<void> addUser(User user) {
  home.state = [...user, state]; // but async
  recent.state = [...user, state];
  // other views don't need to update like 'byId()'
}

@rrousselGit
Copy link
Owner

Exposing the state of all providers like that is fairly problematic as it's not reactive. This doesn't work:

@riverpod
Future<List<User>> home() async {
  return this.recentState;
}

@riverpod
Future<List<User>> recent() => ...;

And also, keep in mind that mutations don't return void.
The syntax is instead:

@mutation
int increment() => state + 1;

Returning void /Future<void> means that's not really a mutation

@rrousselGit
Copy link
Owner

For now I'm leaning toward relying on Ref for updates

So:

@someAnnotation
class Users extends _$Users {
  @riverpod
  Future<User?> byId(String id) => /* http.get(...) */;

  @riverpod
  Future<List<User>> home() => /* http.get(...) */;

  Future<void> addUser(User user) async {
    ref.read(homeProvider._notifier).state = [ref.read(homeProvider._notifier).state, user];
  }
}

But that's pretty bad syntax wise.

And mutations have the added issue that we don't know which state a mutation is associated to.

We'd need something like:

  @Mutation(homeProvider)
  Future<List<User>> addUser(User user) async {
    return [ref.read(homeProvider._notifier).state, user];
  }

That's not ideal either.

@snapsl
Copy link

snapsl commented Dec 5, 2024

Exposing the state of all providers like that is fairly problematic as it's not reactive.

Can you elaborate this.
Dependant states could use ref.watch during init.

@riverpod
class Users {
  @reactive
  Future<User?> byId(String id) => /* http.get(...) */;

  @reactive
  Future<List<User>> home() => ref.watch(usersProvider.recent.where((a) => a.isHome == true)); // it's like a computed value
  
  // alternative
  @computed
  Future<List<User>> home() => this.recent.where((a) => a.isHome == true)); // this is reactive due to @computed

  @reactive
  Future<List<User>> recent() => /* http.get(...) */;

  @mutation
  int addUser(User user) => /* ... */;    
}

And also, keep in mind that mutations don't return void.

This should not throw in a callback.

onPressed: () => addUser(user)

If this can be avoided a mutation can also return a mutation state.


And mutations have the added issue that we don't know which state a mutation is associated to.

I think a mutation should apply to the bundle of user provider and thus to each of its dependent "view" providers. Defining a mutation that only applies to the homeProvider is redundant for a bundled provider, as it should then be a separate provider.
Even tough it would still be possible.

@mutation
Future<List<User>> addHomeUser(User user) async {
  home.state = [...user, home.state]; // but async
  ...
}

@rrousselGit
Copy link
Owner

Can you elaborate this.
Dependant states could use ref.watch during init.

The problem is that folks can make the mistake and use this.homeState instead of ref.watch(homeProvider).
It's not a problem when using Riverpod normally.

This should not throw in a callback.

onPressed: () => addUser(user)

If this can be avoided a mutation can also return a mutation state.

I see no relation between this snippet and the fact that mutations return T instead of void. Could you clarify?
When you call addUser(user), you're not calling the method directly. Riverpod calls it on its own. There's a wrapper around it.

Defining a mutation that only applies to the homeProvider is redundant for a bundled provider, as it should then be a separate provider.

It can't be separate. The goal of this feature is to allow folks to synchronise independent providers when they manage the same object.
If we extract home as a separate provider, it won't be synchronised

@rrousselGit
Copy link
Owner

rrousselGit commented Dec 5, 2024

Another alternative I was considering is instead:

const userGroup = Group<User>();

@riverpod
@userGroup
User? byId(Ref ref, String id) => ...;

@riverpod
@userGroup
class Home {
  @override
  Future<List<User>> build() => ...
}

@snapsl
Copy link

snapsl commented Dec 5, 2024

I see no relation between this snippet and the fact that mutations return T instead of void. Could you clarify?
When you call addUser(user), you're not calling the method directly. Riverpod calls it on its own. There's a wrapper around it.

I'm referring to #3682 but the wrapper you mentioned should solve this. 👍


Another alternative I was considering is instead:

const userGroup = Group<User>();

@riverpod
@userGroup
User? byId(Ref ref, String id) => ...;

@riverpod
@userGroup
class Home {
  @override
  Future<List<User>> build() => ...
}

Looks promising and declarative!
I have not yet seen such an approach and therefore cannot imagine how the synchronization would work.

@rrousselGit
Copy link
Owner

I'm referring to #3682 but the wrapper you mentioned should solve this. 👍

No.
The wrapper will rethrow any error thrown by your mutation.

Don't throw in your mutations if you don't want Riverpod to rethrow errors :)

@snapsl
Copy link

snapsl commented Dec 5, 2024

No. The wrapper will rethrow any error thrown by your mutation.

Don't throw in your mutations if you don't want Riverpod to rethrow errors :)

I have not seen mutation implemented that way. Commonly you would return a mutation.error state or <void>.

@rrousselGit
Copy link
Owner

There are two different things here. There is a mutation.error. You can do:

final increment = ref.watch(provider.increment);

switch (increment.state) {
  case IddleMutationState():
  case PendingMutationState():
  case SuccessMutationState(:final state):
  case ErrorMutationState(:final error, :final stackTrace):
}

But we're not talking about that here.

We're talking about how increment.call() throws if the operation fails.
IMO it would be dangerous to silence the error.

@lucavenir
Copy link
Contributor

My two cents about the throw concern, which is kind-of OT on this issue: I'd say "let it fail".
You can easily capture and properly log callbacks' failures.
AFAIK bad things happen if you mute errors, even on "expected errors".

About the Group<T> syntax: it's just perfect.
It looks clear, it's flexible, it's safe, and it also looks easy to document / maintain.
I might be biased, but it's also pretty similar to well-known approaches I'm familiar with, so I'm pretty happy about this proposal.

Looking forward for this feature to see light.

@w0rsti
Copy link
Author

w0rsti commented Dec 6, 2024

My two cents about the throw concern, which is kind-of OT on this issue: I'd say "let it fail".

You can easily capture and properly log callbacks' failures.

AFAIK bad things happen if you mute errors, even on "expected errors".

About the Group<T> syntax: it's just perfect.

It looks clear, it's flexible, it's safe, and it also looks easy to document / maintain.

I might be biased, but it's also pretty similar to well-known approaches I'm familiar with, so I'm pretty happy about this proposal.

Looking forward for this feature to see light.

+1

The group annotation makes it also easier to split the providers into its own files, while the first approach gives me strong vibes of needing to put all the providers into one file which would be fine too but in case of many "related" providers it can get pretty big.

@rrousselGit
Copy link
Owner

rrousselGit commented Dec 6, 2024

Having to put related things together IMO make sense.
It makes things so much easier to find.

The main issue is that Dart lacks nested classes.

Even if I add that Group thing, the day we have nested classes, I'd add the initial approach.

There's a lot of value in being able to do:

ref.watch(users.byId(...))
ref.watch(users.home)
ref.watch(users.search('...'))

That's way more discoverable than having to guess the provider name.

@lucavenir
Copy link
Contributor

There's a lot of value in being able to do:

ref.watch(users.byId(...))
ref.watch(users.home)
ref.watch(users.search('...'))

That's way more discoverable than having to guess the provider name.

Yes, absolutely. This would greatly help maintainability of my codebases atm (:

But why private classes?
Is it possible to let @Group<T> annotation generate something similar, bringing the same value?
e.g.

typedef UsersGroup = Never;
extension Users on UsersGroup {
  static SomeProvider get home => ...;
}

// and then
ref.watch(Users.home);

The above is how the Dart team proposes to namespace static members together. Check it out.
It might sense to follow Dart guidelines in this regard.

The only issue might be riverpod_lint; I'm not sure how it works exactly, but I previously read it could be hard for the linter to work on static providers. Might this be different, because it's inside an extension?

@rrousselGit
Copy link
Owner

Maybe like that?

const group = Group<User>();

@riverpod
@group
class MyNotifier {...}

which generates:

extension on Group<User> {
  get myNotifierProvider => ...
}

Used as:

ref.watch(group.myNotifierProvider)

That could be useful. I'll consider it

@lucavenir
Copy link
Contributor

That's even better!
Love it!!

Back to the topic of this issue, how will this syntax help with syncing providers?

@rrousselGit
Copy link
Owner

The idea is that by specifying a group, Riverpod would perform some kind of combine_latest for all providers of that group

So if you update a user with ID 42, all providers that currently expose such a user receive the same update.

@w0rsti
Copy link
Author

w0rsti commented Dec 9, 2024

That's even better!
Love it!!

Back to the topic of this issue, how will this syntax help with syncing providers?

As far as I understand it will also act as a "group cache".
If home() is a subset of users or paginated response the byId() can lookup the group "state" instead of performing the http request.

@snapsl
Copy link

snapsl commented Dec 9, 2024

I see the following remaining problem with the group syntax.

  • Each provider can react differently to group updates. While one provider adds a new user at the beginning of the list, another adds it at the end, and a third inserts it into a Map<int, User>. More precisely, each provider must define how it reacts to each mutation changing the shared state.
  • Where should we define mutations of the group / Which provider defines the mutation?
  • The “group pattern” (which needs a better name) would require a solid documentation with examples. The previous syntax is more in line with the way other state management solutions solve this problem and therefore should be easier to adapt.

@lucavenir
Copy link
Contributor

About overriding state change

About the "how to react to shared state changes" problem, I would assume there would be an overridable method for each provider subscribing to the Group<T>, allowing to customise how each piece of state updates itself in response to that.

I also have this concern, as my use case has "favorites", in which a state change in my home page - i.e. "the item has been favorited" - triggers a completely different change in my favorited list ("add this element on top of this list, and make sure it's favorited since it wasn't in here before").

Is this assumption correct?

About docs

About the documentation concern: I think that's safe to say Riverpod is and will be well documented (:

Another concern: Group<T> where T is...?

Another topic would be about synchronising slightly different types.

Here's my use case: I have BookSnippet and BookDetails; they are returned by .home() and by .byId() providers respectively.

Indeed, BookSnippet is a subset of BookDetails, but besides this, how would I specify my Group at that point?
I guess I can define a common interface, e.g. BookInterface, let the concrete implementations implement it, and then create a Group<BookInterface>.

My use case would be 100% covered, but I'm not sure if some folks have different requirements (e.g. entities that can't share the same interface).

@snapsl
Copy link

snapsl commented Dec 9, 2024

I also have this concern, as my use case has "favorites", in which a state change in my home page - i.e. "the item has been favorited" - triggers a completely different change in my favorited list ("add this element on top of this list, and make sure it's favorited since it wasn't in here before").

No "magic" should happen.
It depends on how you define your mutations and how each provider should react to these changes. (see above)


Another concern: Group where T is...?
Another topic would be about synchronising slightly different types.

This may be a limitation of the group syntax.
For synchronization, it does not matter whether the return type is List<Book>, Map<int, Book> or BookSnippet, as long as you can define how each provider should react to changes of the synchronized value.


In general: Mutation <-> Reaction is a NxM problem. Possibly the only way riverpod can support here is to enforce that every connection is defined.

@rrousselGit
Copy link
Owner

rrousselGit commented Dec 9, 2024

The goal of Group would be to be a high level API, that's simple to use but possibly limited.
It would be combined with some lower-level APIs, that are harder but with a boarder scope.

Specifically, I was thinking that:

  • @Group<T> could only be specified on a provider if its state is either T, List<T> or Map<T, ...>/Map<..., T> .... or substates like T? and more.
  • Specifying one Group<T> automatically syncs all T instances within the group (based on T.id if present. If not, you have to specify a field name for merging fields. Like @Group<User>(idName: 'my_id')
  • Riverpod would synchronise values based on what is the "latest"

This would work by having Riverpod observe all providers within a group. And whenever a emits an update, it'd look for the ID of the updated item. Then, it'd search within other providers of the same group for other items with the same ID. Lastly, when there's a match, it'd update those providers to use the new value (either with mutation or copy)

As such, this is a complete example:

class User {
  User(this.id);
  final String id;
}

const userGroup = Group<User>();

@riverpod
@userGroup
User? byId(Ref ref, String id) => ...;

@riverpod
@userGroup
class Home {
  @override
  Future<List<User>> build() => ...
}

Any update on either byId or Home would be propagated to the other one, if the update is applied on an ID they have in common.

Overall, this would probably be a bit magical.
But it'd also be difficult to not have a "magical" API yet stay simple.

We'd also probably have to prevent providers within a group from watching each-other. Because that'd easily end-up in an infinite rebuild loop.

About the "how to react to shared state changes" problem, I would assume there would be an overridable method for each provider subscribing to the Group, allowing to customise how each piece of state updates itself in response to that.

We can have an overridable method on notifiers for that.

I'd expect it to look like:

@override
void handleGroupUpdate(Iterable<AsyncValue<T>> updates) {
  // TODO update your state based on those changes
}

It'd probably always use AsyncValue even if your notifier is synchronous.
Because AsyncValue has extra properties. For example when using offline persistence, there's an AsyncValue.isFromCache

@lucavenir
Copy link
Contributor

Thank you! This is awesome! I would assume this is a lot of work tho.

This won't ship with Riverpod 3.0, right?

@rrousselGit
Copy link
Owner

We'll see. In terms of difficulty, it's alright. We already have all the pieces necessary to implement this.
Heck this could probably be implemented as a separate package.

The main issue here is about "Is that proposal a good idea?"
It sounds cool on paper, but I don't know all the impacts that this could cause.

If I release it, I'll likely ship the initial release as "experimental"

@rrousselGit
Copy link
Owner

By the way, if someone fancies trying to work on this, I'm willing to get a PR.
So long that:

  • it targets the dev branch
  • any public API is exported through either package:riverpod/experiment.dart or package:riverpod_annotation/experimental.dart

For now I'd like to finish offline and a lot of other features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

7 participants