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

Using "actors" or event dispatcher #289

Closed
Chojecki opened this issue Jan 25, 2021 · 34 comments
Closed

Using "actors" or event dispatcher #289

Chojecki opened this issue Jan 25, 2021 · 34 comments
Labels
enhancement New feature or request

Comments

@Chojecki
Copy link

Chojecki commented Jan 25, 2021

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.

@Chojecki Chojecki added documentation Improvements or additions to documentation needs triage labels Jan 25, 2021
@rrousselGit
Copy link
Owner

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

@rrousselGit rrousselGit added enhancement New feature or request and removed needs triage documentation Improvements or additions to documentation labels Jan 25, 2021
@Chojecki
Copy link
Author

Chojecki commented Jan 25, 2021

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
Copy link
Owner

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
Copy link
Owner

rrousselGit commented Jan 30, 2021

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
Copy link
Owner

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

@Chojecki
Copy link
Author

Chojecki commented Jan 30, 2021

@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
Copy link
Owner

rrousselGit commented Jan 30, 2021

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:

  • have two "event"
  • one of the event takes a parameter passed from the UI

@Chojecki
Copy link
Author

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

@rrousselGit
Copy link
Owner

rrousselGit commented Jan 30, 2021

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
Copy link
Author

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
Copy link
Owner

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

@smiLLe
Copy link

smiLLe commented Jan 31, 2021

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
Copy link
Owner

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
Copy link

smiLLe commented Jan 31, 2021

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
Copy link

smiLLe commented Jan 31, 2021

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
Copy link
Owner

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
Copy link
Owner

rrousselGit commented Jan 31, 2021

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
Copy link
Owner

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
Copy link

smiLLe commented Jan 31, 2021

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
Copy link
Owner

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
Copy link

smiLLe commented Jan 31, 2021

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
Copy link
Author

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
Copy link
Owner

@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
Copy link

smiLLe commented Jan 31, 2021

@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
Copy link

smiLLe commented Jan 31, 2021

@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
Copy link
Author

@smiLLe thanks. How would you call event later?

@smiLLe
Copy link

smiLLe commented Jan 31, 2021

@Chojecki

Button(tap: () => context.read(Post.fetchPosts)());

@smiLLe
Copy link

smiLLe commented Jan 31, 2021

@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
Copy link
Author

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

@smiLLe
Copy link

smiLLe commented Jan 31, 2021

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

@Chojecki
Copy link
Author

@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
Copy link

smiLLe commented Jan 31, 2021

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

@rrousselGit
Copy link
Owner

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

Feel free to try and implement it 😄

For now my focus in on writing a devtool.

@rrousselGit
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants