intendednull / yewdux

Ergonomic state management for Yew applications
https://intendednull.github.io/yewdux/
Apache License 2.0
319 stars 31 forks source link

Middleware? #40

Closed ivmarkov closed 1 year ago

ivmarkov commented 1 year ago

Interesting project!

I'm looking to retire my buggy home-grown copy of Redux in favor of something much more refined.

Where I'm a bit stuck is that it seems the yewdux design is lacking the notion of an "Action", and therefore I'm not sure that I can also retire my home-grown notion of Redux middleware.

Was the Redux action pattern eradicated in the yewdux design for less boilerplate, or for other reasons? (Obviously, I need to somehow recover it on top of yewdux if I am to support a proper middleware abstraction.)

alilee commented 1 year ago

Is this achieved by implementing Reducer<Store> for your action object? (see Mutating state predictably here)

ivmarkov commented 1 year ago

Is this achieved by implementing Reducer<Store> for your action object? (see Mutating state predictably here)

Good point. Yes, it seems Msg can fill-in the role of an Action for me. I still need to develop a custom dispatch function that takes a store instance + an action instance (or Msg instances if you wish) though and thread it down to all components using e.g. use_context or something similar. Or am I missing something here as well?

alilee commented 1 year ago

These plain old dispatchers will do the job. The code snippet I linked omits how they are created but it is shown above on the same page.

let dispatch = Dispatch::<S>::new();           // S impls Store
// or often more conveniently
let (_state, dispatch) = use_store::<S>();

dispatch.apply(Msg::DoIt);      // immediately
// or 
let callback = dispatch.apply_callback(|_| Msg::DoIt);     // for use as a callback, eg. onclick
ivmarkov commented 1 year ago

These plain old dispatchers will do the job. The code snippet I linked omits how they are created but it is shown above on the same page.

let dispatch = Dispatch::<S>::new();           // S impls Store
// or often more conveniently
let (_state, dispatch) = use_store::<S>();

dispatch.apply(Msg::DoIt);      // immediately
// or 
let callback = dispatch.apply_callback(|_| Msg::DoIt);     // for use as a callback, eg. onclick

Are we talking about the notion of middleware here at all?

How are the above examples of plain old yewdux dispatchers solving this:

I still need to develop a custom dispatch function that takes a store instance + an action instance (or Msg instances if you wish) though and thread it down to all components using e.g. use_context or something similar. Or am I missing something here as well?

My point is: with middleware in place, your components are not even aware that the middleware exists. All they do is to dispatch actions to the store using a dispatcher function they somehow get (either form context, or threaded from props or - the best possible solution - directly from the store, if it was possible to monkey-patch the store dispatcher to become middleware-aware). The fact that there is a middleware that intercepts these dispatches and does whatever (logs the actions, does async calls to the server, you name it) is hidden from them. All the components know about is the dispatch function.

alilee commented 1 year ago

Is it possible that you are missing that Stores are singletons? Every Dispatcher for a Store, end-points with the same (logically static) singleton. A compound object accessed through use_selector is merely looking at a part (ie. attribute or collection member) of the singleton to make the decision about refreshing, rather than the whole thing.

alilee commented 1 year ago

Ok, I'm starting to see your point.

alilee commented 1 year ago

I think I would just build a more composable Reducer. I'm presuming you don't actually want components having different behaviours on dispatch of the same message (eg. only log dispatches from some components)? If that's the case, I'd need an intuition as to why.

alilee commented 1 year ago

I totally agree this is an interesting case and I can show you my sketch of this.

ivmarkov commented 1 year ago

I totally agree this is an interesting case and I can show you my sketch of this.

Your sketch is hard-coding the communication with the server inside the reducers themselves. As in here and here. This is as far from the notion of middleware as one can get, sorry. With the middleware concept in place, reducers should just operate on the store, and the communication to the server happens totally outside of the reducers.

To understand the middleware concept, ask yourself the following question: "How would I design my DebugRelationView component in such a way, that the user can "plug" their own network protocol with the server?" As in e.g. using websockets to call the server. Perhaps re-reading the Redux middleware link I pasted might help as well?

intendednull commented 1 year ago

The closest thing we have currently are listeners, however those don't have any context over how state changed, just that it did.

I'm definitely open to middleware support. We'll likely need to approach it differently than Redux, however I don't have a solid solution yet. Ideas and suggestions are welcome!

ivmarkov commented 1 year ago

Right. I don't see any other good place to plug a middleware notion besides in between the actions and their application to the store (i.e. middleware should intercept the dispatch to the store). Which however would mean that actions need to become simple data carriers as I mentioned here rather than being potentially opaque store-mutating function closures as yewdux currently allows. Because the only "context change" signal a middleware has is the data in the action itself. So it needs to be capable of "peeking" into the action data so as to e.g. log it or send it over the wire. By the way, these mutating closures are not buying us much in terms of memory footprint I think - as they need to close over their environment, so I suspect their payload would be as big as their simple data carrier actions' equivalents.

The above would probably mean that it is cleaner if yewdux reverses the Reducer pattern and re-applies it onto the store rather than - as it currently is - on the actions themselves. Which in turn means a lot of breaking changes. Add to that disallowing of store mutating function closures, and perhaps we are ending up with a different framework?

I mean, a "dispatch-intercepting" middleware can be built with minimal changes to the existing yewdux approach, but it won't be very capable if the user is not consciously modeling their changes to the stores in the form of actions/reducers which do have publically-accessible payload (i.e. what you call "messages" and which are mutating the store via apply / apply_callback). Perhaps putting this in big bold letters somewhere in the documentation, and then making changes to the dispatch mechanism, so that user-provided middleware can hook in there would be a good-enough initial step?

ivmarkov commented 1 year ago

I mean, a "dispatch-intercepting" middleware can be built with minimal changes to the existing yewdux approach, but it won't be very capable if the user is not consciously modeling their changes to the stores in the form of actions/reducers which do have publically-accessible payload. Perhaps putting this in big bold letters somewhere in the documentation, and then making changes to the dispatch mechanism, so that user-provided middleware can hook in there would be a good-enough initial step?

A step even before that would be not to change yewdux but to just wrap the dispatching. I.e. instead of:

struct Msg {
    AddOne,
}

impl Reducer<Counter> for Msg {
    fn apply(&self, mut counter: Rc<Counter>) -> Rc<Counter> {
        let state = Rc::make_mut(&mut counter);

        match self {
            Msg::AddOne => state.count += 1,
        };

        counter
    }
}

#[derive(Debug, Default, Clone, PartialEq, Eq, Store)]
struct Counter {
    count: u32,
}

#[function_component]
fn App() -> Html {
    let (counter, dispatch) = use_store::<Counter>();
    let onclick = dispatch.apply(Msg::AddOne);

    html! {
        <>
        <p>{ counter.count }</p>
        <button {onclick}>{"+1"}</button>
        </>
    }
}

user would have to write:

// Msg and Counter definition skipped for brevity, but they stay unchanged

#[function_component]
fn App() -> Html {
    let (counter, dispatch) = use_store::<Counter>();
    let middleware = Middleware::hook(dispatch); // <-- Hook the dispatch with the middleware framework so that all middleware provided by the user can inspect the `Msg` action prior to / after it is applied to the store
    let onclick = middleware.apply(Msg::AddOne);

    html! {
        <>
        <p>{ counter.count }</p>
        <button {onclick}>{"+1"}</button>
        </>
    }
}

or with a custom hook that just wraps use_store:

// Msg and Counter definition skipped for brevity, but they stay unchanged

#[function_component]
fn App() -> Html {
    let (counter, middleware_dispatch) = use_store_with_middleware::<Counter>(); // Just a wrapper that uses `use_store` and wraps the returned `Dispatch` in `Middleware`. How to name this new "hook" is another topic...
    let onclick = middleware_dispatch.apply(Msg::AddOne);

    html! {
        <>
        <p>{ counter.count }</p>
        <button {onclick}>{"+1"}</button>
        </>
    }
}

but that's acceptable IMO. And the middleware impl would sit completely on top of yewdux. And we can constrain the Middleware struct impl to only have the middleware-friendly apply / apply_callback methods and not have the reduce_* stuff which takes closures.

I can try that.

intendednull commented 1 year ago

I think keeping it as a separate (optional) layer that sits on top of the normal dispatch is a great idea. We're really trying to remain as flexible as possible, and doing it that way will add capabilities, without taking any away.

Feel free to take charge here. My only suggestion is to try making this a separate crate, like crates/yewdux-middleware. This way the boundary is clear.

Thanks for your interest! Looking forward to progress :)

intendednull commented 1 year ago

Another change I've been considering is breaking the different dispatch methods into dedicated traits, so they can be selected at will, without exposing unused or unwanted interfaces. This seems fairly relevant here as well

intendednull commented 1 year ago

The above would probably mean that it is cleaner if yewdux reverses the Reducer pattern and re-applies it onto the store rather than - as it currently is - on the actions themselves

The current pattern is intentially designed to be action-generic. Meaning stores can apply any action that is implemented to do so. One action type per store can be rather limiting, especially for larger applications.

I don't see this design limiting middleware, other than having to take a slightly different approach. Correct me if I'm wrong, of course. Here is a good place to put any conflicts you find.

ivmarkov commented 1 year ago

I think keeping it as a separate (optional) layer that sits on top of the normal dispatch is a great idea. We're really trying to remain as flexible as possible, and doing it that way will add capabilities, without taking any away.

Feel free to take charge here. My only suggestion is to try making this a separate crate, like crates/yewdux-middleware. This way the boundary is clear.

Thanks for your interest! Looking forward to progress :)

Initial implementation here. Warning: no docs or tests yet. And a usage pattern here.

The way this is supposed to work is that the user should essentially forget about the dispatch module of yewdux itself, and instead use the dispatch module of yewdux-middleware.

Btw, I'm having weird issues with anymap 0.8 in that it panics on me. Including in yewdux 0.8 itself. Have you seen that?

ivmarkov commented 1 year ago

Another change I've been considering is breaking the different dispatch methods into dedicated traits, so they can be selected at will, without exposing unused or unwanted interfaces. This seems fairly relevant here as well

This might be a starting point then. Note that I've renamed apply to invoke as apply sounds as if the dispatch is going to "apply" this change to some store. Which is not necessarily the case. Look at this and this - dispatchers that don't really update any store.

ivmarkov commented 1 year ago

The above would probably mean that it is cleaner if yewdux reverses the Reducer pattern and re-applies it onto the store rather than - as it currently is - on the actions themselves

The current pattern is intentially designed to be action-generic. Meaning stores can apply any action that is implemented to do so. One action type per store can be rather limiting, especially for larger applications.

I don't see this design limiting middleware, other than having to take a slightly different approach. Correct me if I'm wrong, of course. Here is a good place to put any conflicts you find.

For the middleware dispatch concept to work, your actions (that is - messages) should have a public state. And you should use messages, not really opaque closures. (UPDATE: not really true! This is now enforced in yewdux-middleware by the Dispatch trait). My point is - it is not enforced in yewdux itself but oh well, I can live with that.

intendednull commented 1 year ago

Btw, I'm having weird issues with anymap 0.8 in that it panics on me. Including in yewdux 0.8 itself. Have you seen that?

Haven't had any problems myself, but maybe try anymap2?

ivmarkov commented 1 year ago

By the way, I would appreciate if you take a look at my stuff. :)

I would also like to publish a very initial version on crates.io soon, as I have a bunch of crates - some of these already published on crates.io and some that I want to publish soon that in the meantime depend on it - but I can't as yewdux-middleware itself is not published yet.

Which brings the question - where shall I host this project? I don't mind hosting it in my own github user space, but I'll take the yewdux-middleware name in crates.io that you might want to use in future?

On the other hand, you might feel uncomfortable pulling this code into the yewdux github repo at such an early stage, and - it might lower my velocity, as I'll then have to PR every single change to you?

Perhaps we can have a "gentlemen"'s agreement of sorts that I'll host it myself, and take the yewdux-middleware name on crates.io, but if you decide to pursue your own middleware, I can give you back the crates.io name, as I'll anyway then switch to your middleware, if it ever sees the daylight?

ivmarkov commented 1 year ago

Btw, I'm having weird issues with anymap 0.8 in that it panics on me. Including in yewdux 0.8 itself. Have you seen that?

Haven't had any problems myself, but maybe try anymap2?

The problem is - as I said:

Including in yewdux 0.8 itself

As in yewdux 0.8 not working for me at all and crashing at startup time in anymap. Is it possible to release a patch-level update to 0.8 with anymap updated to - say - anymap2? I'm not ready to take another set of master dependencies in the form of yew 0.20 and yewdux master as of yet.

intendednull commented 1 year ago

Can you share the error? Maybe a minimal reproduction?

It's looking good so far! My only note is the naming for Dispatch and use_store currently clashes with yewdux. Might be a little confusing, I could see users having trouble when trying to import both crates at the same time. Maybe use_store_value and MiddlewareDispatch?

As for hosting, feel free to publish yourself. Honestly not yet convinced middleware is the best approach, but if there's enough interest I'm not completely against the idea either. With async already supported, I struggle to see a problem that couldn't be solved currently, but I could definitely be wrong. Merging is always an option in the future.

ivmarkov commented 1 year ago

With async already supported, I struggle to see a problem that couldn't be solved currently, but I could definitely be wrong. Merging is always an option in the future.

The one, single problem I need middleware for is to be able to "plug" different type of client-to-server communication protocols. As in:

There are other solutions of course, as in just abstracting away only the client-server protocol. But the moment you make the protocol a singleton, you end up with something that suspiciously like middleware anyway. Anyway, let's have this stuff separate from yewdux for now.

I'll try to provide a repro for my anymap struggles tmr. And will open another issue for that.

intendednull commented 1 year ago

Just added AsyncReducer, which also fills this role a bit: https://github.com/intendednull/yewdux/tree/master/examples/async_reducer