intendednull / yewdux

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

Have a way to inject listeners into derived `Store` implementation #51

Closed wainwrightmark closed 1 year ago

wainwrightmark commented 1 year ago

Hi. I love this crate - I use it in all my yew projects.

At the moment it's a bit of a pain to use listeners because I am almost always using the Derive attribute to implement Store and persist to local storage. If I want to add a listener to a Store with zero listeners I need to expand the macro and add the init_listener statements. This is a bit ugly and potentially error prone. It would be nice to have an attribute to initialize extra listeners in the generated code.

For example you could write something like:

use yewdux::{store::Store, prelude::Listener};
pub struct ExampleListener;

impl Listener for ExampleListener{
    type Store = ExampleStore;

    fn on_change(&mut self, state: std::rc::Rc<Self::Store>) {
        log::info!("Changed")
    }
}

#[derive(Default, PartialEq, Store)]
#[store(storage = "local", storage_tab_sync )]
#[store_listener(ExampleListener)]
pub struct ExampleStore{
    prop: bool
}

Instead of:

use yewdux::{store::Store, prelude::Listener};
use yewdux::prelude::init_listener;
pub struct ExampleListener;

impl Listener for ExampleListener{
    type Store = ExampleStore;

    fn on_change(&mut self, state: std::rc::Rc<Self::Store>) {
        log::info!("Changed")
    }
}

#[derive(Default, PartialEq)]
pub struct ExampleStore{
    prop: bool
}

impl Store for ExampleStore {
    #[cfg(not(target_arch = "wasm32"))]
    fn new() -> Self {
        init_listener(ExampleListener);
        Default::default()
    }

    #[cfg(target_arch = "wasm32")]

    fn new() -> Self {
        init_listener(StorageListener);
        init_listener(ExampleListener);

        storage::load(storage::Area::Local)
            .ok()
            .flatten()
            .unwrap_or_default()
    }

    fn should_notify(&self, other: &Self) -> bool {
        self != other
    }
}

struct StorageListener;
impl Listener for StorageListener {
    type Store = ExampleState;

    fn on_change(&mut self, state: Rc<Self::Store>) {
        #[cfg(target_arch = "wasm32")]
        save(state.as_ref(), Area::Local).expect("unable to save state");
    }
}

Happy to have a go and do a PR if you think this is a good idea.

intendednull commented 1 year ago

Great idea! Feel free to open a PR

wainwrightmark commented 1 year ago

Am working on implementing this. However I've run into a bit of a problem - the code I posted above doesn't work - it seems like you can only have one listener for each Store - in the book it says

NOTE: Successive calls to init_listener on the same type will replace the existing listener with the new one.

but I took that to mean that you can't register the same listener twice - not that you can only have one type of listener for each Store. Is that intended? It seems like it would be nice to have more than one (that's the whole point of this issue :) )

If it's not intended or you want it changed I can have a go but that's a slightly bigger job and might be a breaking change...

intendednull commented 1 year ago

Are you getting an error? You should definitely be able to have multiple types of listeners for one store

wainwrightmark commented 1 year ago

No error. It's just the first listener is dropped when the second listener is registered. Try out this example: https://github.com/wainwrightmark/yewdux/tree/multiple-listeners/examples/multiple_listeners

I believe it should be relatively easy to fix - just make ListenerStore generic over the type of listener rather than the type of store.

intendednull commented 1 year ago

That's definitely a bug! Yes you're correct, that would fix it. Feel free to include it in your PR, and maybe a unit test too?