rrousselGit / riverpod

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

Using "actors" or event dispatcher #289

Closed Chojecki closed 3 years ago

Chojecki commented 3 years ago

Hi, the more time I'm spending with Riverpod, the more I'm removing StateNotifier and StateNotifierProvider for just FutureProvider and StreamProvider. It works from me a little like react-query with hooks in one in react word.

Can't find a good answer for my question: how can we deal with events? I mean something like:

  1. We have a list of Posts.
  2. Using FutureProvider to get all Posts IDs from remote database
  3. Using StreamProvider to watch every particular Post (likes, comments etc.)
  4. Want to send an event to data base to like one of the post, so my StreamProvider from point above, will notify about Post change and will update the data of this particular Post.

So what is best practice to send this event of like/unlike the Post? Normally I would have a Repository, StateNotifier which takes this Repository and have a method likeOrDisllike. Then I can use something like final controller = useProvider(thisStateNotifierProvider) and on like click do controller.likeOrDislike(). But as I said, with good of Future/Stream Provider I see, there might be 95% of my CRUD app logic with just this two.

I was thinking about Providing the method from Repository like:

final actor =
  Provider.family.autoDispose<Future<void>, String>((ref, postId) async {
  final repo = ref.watch(firebaseSpotRepositoryProvider);
  final user = ref.watch(userProvider);
  await repo.likeOrDisslikePost(userId: user.id.getOrCrash(), postId: postId);
});

The on "UI" :

Button(
    onTap: () async {
              context.read(
                actor(
                  post.id.getOrCrash(),
                ),
          );
      },
)

Does it make sense? Or is there a better approach or it's super stupid and whole thinking about resign from any class controllers like StateNotfier is wrong.

rrousselGit commented 3 years ago

There's at the moment no built-in event mechanism.
I'm thinking about it for the same reasons you mentioned. But I have yet to come up with an API that satisfy my tastes.

I'm open to suggestions though

Chojecki commented 3 years ago

Happy to think about it more and try to propose something.

What do you think about the workaround I did above? Not as a decent solution, but do you see any weaknesses there - Provider with family takes the params, and autodispose to allow "onTap" dispatch multiple times for example.

rrousselGit commented 3 years ago

I'm not sure about your code

The way I've done it is by having a separate Provider that exposes a StreamController:

final eventProvider = Provider<T>((ref) {
  final controller = StreamController<T>();
  ref.onDispose(controller.close);

  return controller;
});

final postProvider = StreamProvider.family<Post, String>((ref, id) async* {
  // whatever you're doing inside your StreamProvider without the event mechanism
  yield await doSomething();

  // the event mechanism
  final controller = StreamController<Post>();
  ref.onDispose(controller.close);

  final sub = ref.watch(eventProvider).listen((event) {
    controller.add(Post(...));
  });
  ref.onDispose(sub.cancel); 

  yield* controller.stream;
});
rrousselGit commented 3 years ago

What I'm thinking of at the moment is:

final eventProvider = EventProvider<T>();

final postProvider = StreamProvider.family<Post, String>((ref, id) async* {
  // whatever you're doing inside your StreamProvider without the event mechanism
  yield await doSomething();

  // the event mechanism
  ref.listen<Event>(eventProvider, (event) {
    ref.setState(AsyncValue.data(Post(...)));
  });
});

...

context.dispatch(eventProvider, Event(...));
rrousselGit commented 3 years ago

cc @smiLLe because I know you've worked on something similar

Chojecki commented 3 years ago

@rrousselGit not sure if I understood your idea there. I mean, where can I call a method from some Repository class there? Maybe I just don't understand Riverpod enough.

So my concept idea was to something like this:

final someProvider = FutureProvider.mutated<T>((ref) async {

   final repository = ref.watch(someRepositoryProvider)

    // mutation is taken from Provider/FutureProvider/StreamProvider. 
    // someMetodReturnedFutureOfT could be Future<List<Post>> updateData(Post post)
   final actor = mutation(repository.someMetodReturnedFutureOfT);

   // if we want more than one actor.act
   final secondActor = mutation(repository.something);

   // mutated gives us the cache - last provider return data
   final lastData = cache

   // onSuccess or onError will be returned as new provider data for example Future of what onSuccess returns after mutation call in Widget
   actor.act(
      onSuccess: (T data) {
           final lastSuccessData = await cache
           // manipulate the data
           return someData;
       } ,
      onError: () => cache;
   )

  secondActor.act(
      onSuccess: (T data) {
           final lastSuccessData = await cache
           // manipulate the data
           return someData;
       } ,
      onError: () => cache;
   )

   final futureData = repository.getSomeFutureData;
   return futureData;
});

Now someProvider exposes mutation and later for example in HookWidget we can:

// some code
// mutation has the same type like the method minded to actor, so Function(Post)
final mutation = useProvider(someProvider.mutation);

...

Button(onTap: mutation(post), ...

No idea if does it make sense :)

Edited ☝️

rrousselGit commented 3 years ago

I mean, where can I call a method from some Repository class there?

Inside the listen:

final sub = ref.watch(eventProvider).listen((event) async {
  final repository = ref.read(repository);
  await respository.doSomething();
  controller.add(Post(...));
});

For example you could implement a full counter:

// the class is just a namespace
abstract class Counter {
  static final provider = Provider<int>((ref) {
    increment.listen(ref, () {
      ref.setState(ref.state + 1);
    });
    decrement.listen(ref, () {
      ref.setState(ref.state - 1);
    });

    setTo.listen(ref, (value) {
      ref.setState(value);
    });

    return 0;
  });

  static final increment = Event();
  static final decrement = Event();
  static final setTo = UnaryEvent<int>();
}

Widget build(context, watch) {
  final count = watch(Counter.provider);

  return Column(
    children: [
      Text('$count'),
      RaisedButton(
        onPressed: () => Counter.increment(context);
        child: Text('increment'),
      ),
      RaisedButton(
        onPressed: () => Counter.setTo(context, 42);
        child: Text('set to 42'),
      ),
    ],
  );
}

I sadly don't understand your code.

Do you mind updating your snippet to:

Chojecki commented 3 years ago

I edited my comment as you requested, but reading your reply, original answer makes more sense to me.

rrousselGit commented 3 years ago

I still don't understand your code

How does the onTap knows whether to call the first or second actor? What are those onSuccess/onError parameters? What is myProvider.mutation? How is it linked to the actors?

Could you make a full example like I did?

Chojecki commented 3 years ago

You are right. Didn't notice that onTap don't know which method must be called.

It's just a raw idea of exposure mutation by provider and this mutation could accept Future function, so by calling this mutation, act is called and resolves.

Probably wrote it without think about it enough, so it has lacks. Will try to rethink and come back with full example

rrousselGit commented 3 years ago

What do you think of my example then? As I'm confident that it can be implemented

smiLLe commented 3 years ago

Hi @rrousselGit

I was trying to add some event logic. I also came up with your abstract Counter example. The reason i dislike this approach comapred to:

class AsClassCounter extends StateNotifier<int> {
  void  increment() {}
  void  decrement() {}
  void  setTo(int i) {}
}

is this:


Widget build(context, watch) {
 /// lets remove this part
 /// final count = watch(Counter.provider);

  return Column(
    children: [
      Text('$count'),
      RaisedButton(
        onPressed: () {  
          /// This will not update the Provider<int>() because we have never used watch(Counter.provider);
         /// So Provider<int>() does not exist, yet.
          /// For me, this is kind of a logic bug where people MUST understand that increment
          /// will never create Provider<int>() but Provider<int>() will create increment.
          Counter.increment(context); 

         /// This will update the provider. This works as expected
         context.read($AsClassCounter).increment();
        }
        child: Text('increment'),
      ),
      RaisedButton(
        onPressed: () => Counter.setTo(context, 42);
        child: Text('set to 42'),
      ),
    ],
  );
}
rrousselGit commented 3 years ago

Hum that's a good point.

Then I guess the alternative is to add a ".event" modifier

But that'll require using union types for the event, which is tedious without Freezed.

Another issue is probably that we're starting to get quite a lot of generic parameters:

Provider.family<Result, Parameter, Event>((ref, Parameter param) {
 ref.onEvent((Event event) {

 });

  return Result();
})
smiLLe commented 3 years ago

Imagine having 5+ Events. This will become a very large .onEvent callback. And how do we nicely solve async without .then() nesting etc.? This will certainly result in people writing FutureProviders and ref.container.refresh($futProvider) them in .onEvent. No big advantage over a simple StateNotifier imo or am I missing sth?

smiLLe commented 3 years ago

This was some riverpod abstraction idea I was working on the last few months.

final fooStore = StoreProvider<String>((_) => 'foo);
final setString = fooStore.reducer<String>((fooStoreState, payload) => payload);
final asyncSetString = fooStore.effect<String, Payload>(handler: (ref) {
  final dio = ref.watch($dio);
  return (Payload p) => dio.get(''); } ,
 reduce: (fooStoreState, payload) {
 payload.when(loading: () => fooStoreState, error: (_) => fooStoreState, done: (val) => val);
});

An Effect is a StateNotifier<EffectValue<T>>. During creation you get ref in the handler callback. And handler will return a function which will be called like this (for example in a Widget build) context.read(asyncSetString).call('payload').

An 'Reducer' is basically a Event. It is a StateProvider<Payload>.

If you can see, both, setString and asyncSetString are created by fooStore. So they are "bound" to that store. This will resolve the problem described above: Counter.increment. Reading context.read(setString) will create fooStore and will reduce the fooStore to a new state. Watching fooStore will never create setString because you don't have a need for setString at this point in your application.

rrousselGit commented 3 years ago

Imagine having 5+ Events. This will become a very large .onEvent callback.

I don't think so.

We could have:

@freezed
abstract class Event with _$Event {
  factory Event.increment() = Increment;
  factory Event.setTo(int value) = SetTo;
}

Provider.event<Event>((ref) {
 ref.onEvent<Increment>((Increment event) {

 });

  ref.onEvent<SetTo>((SetTo event) {

  });

  return Result();
})

And how do we nicely solve async without .then() nesting etc.?

I'm not sure I understand. Why can't we use async/await?

rrousselGit commented 3 years ago

Alternatively, to fix the issue you mentioned before with the Counter.increment not mounting the counter provider, we could do:

abstract class Counter {
  static final provider = ...;

  static final increment = VoidEvent(provider);
}

This way when we dispatch the event, this will automatically mount the provider if it wasn't mounted already.

This may even improve performances quite a bit since it doesn't require having a "listening" mechanism for events. The event would directly be dispatched to the associated provider

rrousselGit commented 3 years ago

I'd prefer having the event handling inside the provider:

Provider((ref) {
  onEvent((_) {
    // ...
  });
}

instead of:

Provider((ref) {

});

Event((event) {
  ...
});

because it is significantly more flexible as it allows the event to have access to all the local variables of the provider.

For example it allows:

final provider = FutureProvider<Something>((ref) async {
  final completer = Completer<void>();

  startEvent.listen(() => completer.complete());

  await completer.future;

  return await fetchSomething();
});

final startEvent = VoidEvent(provider);
smiLLe commented 3 years ago

So we could have:

abstract class Counter {
  static final state = State<int>((ref) {
    ref.on(increment, () {
      ref.reduce((state) => state + 1);
    });

    ref.on<int>(add, (amount) {
      ref.reduce((state) => state + amount);
    });

    return 0;
  });

  static final increment = VoidEvent(state);
  static final add = Event<int>(state);
}

EDIT:

Widget build(c) {
  final counterState = useState(Counter.state);
  return Text(counterState);
}
rrousselGit commented 3 years ago

Yeah I think that's the best compromise

Creating events is a bit more verbose, but it's still reasonable. And I like the readability of watch(Counter.provider) and Counter.increment(ref)

That'd give a definitive solution to "how to name providers to avoid name clash"

smiLLe commented 3 years ago

Do you see this as part of riverpod or as a seperate package? What do you think about the naming State over Provider and also they would coexist. State would expose ref.on. and ref.reduce() instead of ref.state =. What about the Effect I mentioned above? It could be an Async Event

/// simplified class to just showcase what it is doing
class Effect<T, Payload> extends Event<EffectValue<T>> {
  final Future<EffectValue<T>> Function<Payload>() _handler;
}

abstract class CounteStore {
  static final _initialState = Provider<int>((_) => 0);
  static final state = State<int>((ref) {
    ref.on(increment, () {
      ref.reduce((state) => state + 1);
    });

    ref.on<int>(add, (amount) {
      ref.reduce((state) => state + amount);
    });

    ref.on<EffectValue<int>>(asyncAdd, (asyncVal) {
      ref.reduce((state) => asyncVal.maybeWhen(done: (val) => state + val), orElse: () => state);
    });

    return ref.watch(_initialState);
  });

  static final increment = VoidEvent(state);
  static final add = Event<int>(state);
  static final asyncAdd = Effect<int, int>((ref) => (pl) async => Future.value(pl));
}
Chojecki commented 3 years ago

What do you think of my example then? As I'm confident that it can be implemented

It all make sense and good discussion here. I really would like to see some full example of your idea, because it's looks very promising when I see simple example of Provider, but when Im thinking about some FutureProvide example it just don't "click" in my head. But I guess it's a matter trying it on my own

rrousselGit commented 3 years ago

@smiLLe the event system would be built in Riverpod directly.

I want to keep the api small though. So I'd add the bare minimum: VoidEvent/Event

If you want other things, using extensions/functions should be feasible.

smiLLe commented 3 years ago

@rrousselGit i understand. So the new Api would look like that: Provider<Foo>((ref) {}); Where ProviderReference exposes .onEvent<T>(EventProvider<T> provider, void Function(T payload) {}); and .setState(Foo)?

smiLLe commented 3 years ago

@Chojecki

It all make sense and good discussion here. I really would like to see some full example of your idea, because it's looks very promising when I see simple example of Provider, but when Im thinking about some FutureProvide example it just don't "click" in my head. But I guess it's a matter trying it on my own

One way to do it would be:

abstract class Post {
  static final posts = Provider<AsyncValue<Posts>>((ref) {
    final api = ref.watch(myApi);
    ref.onEvent(fetchPosts, () {
      api.fetchPosts().then((posts) {
        ref.setState(posts);
      });
    });

    return AsyncValue.loading();
  });

  static final fetchPosts = VoidEvent(posts);
}
Chojecki commented 3 years ago

@smiLLe thanks. How would you call event later?

smiLLe commented 3 years ago

@Chojecki

Button(tap: () => context.read(Post.fetchPosts)());
smiLLe commented 3 years ago

@Chojecki I think that you can already do this but the api is more complex:

abstract class Post {
  static final posts =
      StateNotifierProvider<StateController<AsyncValue<Posts>>>((ref) {
    final api = ref.watch(myApi);
    final ctrl = StateController<AsyncValue<Posts>>(AsyncValue.loading());
    ref.onDispose(ref.watch(fetchPosts).addListener((_) {
      api.fetchPosts().then((posts) {
        ctrl.state = posts;
      });
    }, fireImmediately: false));

    return ctrl;
  });

  static final fetchPosts = StateProvider<Null>((_) {});
}

Button(tap: () { context.read(Post.fetchPosts).state = null; } );
Chojecki commented 3 years ago

@smiLLe what is StateController there? A class which extends StateNotifier?

smiLLe commented 3 years ago

@Chojecki It is part of riverpod :) It backs StateProvider

Chojecki commented 3 years ago

@smiLLe ahh ok make sense. I can't see benefits to using it over just StateNotifier as controller and call it as context.read(Post.posts).fetchPosts where fetchPosts is just a method inside StateNotifier class, fetch data and set state as AsyncValue with this API, but happy to try something and follow your progress on it as seems to be a really nice

smiLLe commented 3 years ago

@rrousselGit Feel free to contact me if you lack time and need someone to implement this feature

rrousselGit commented 3 years ago

@rrousselGit Feel free to contact me if you lack time and need someone to implement this feature

Feel free to try and implement it :smile:

For now my focus in on writing a devtool.

rrousselGit commented 3 years ago

Closing as I don't plan on adding an event mechanism for now and would prefer keeping things simple.

But that's something you can build on your own if you want to.