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

allow perform_cmd to send multiple messages #556

Open
WesleyAC opened this issue Nov 9, 2020 · 5 comments
Open

allow perform_cmd to send multiple messages #556

WesleyAC opened this issue Nov 9, 2020 · 5 comments

Comments

@WesleyAC
Copy link

WesleyAC commented Nov 9, 2020

It would be great if perform_cmd had the option of sending multiple messages, as well as just one message. Sometimes I want to use perform_cmd to grab some data from the server, and then trigger multiple messages at once based on that data. Right now, I have to make a new message for each combination of messages, which is just sort of annoying and clunky.

I don't know what the best way of doing this is (Maybe allow perform_cmd to return (), Msg, or Vec<Msg>?), but if someone wants to point me in a good direction, I'd be happy to take a stab at implementing it :)

BTW, Seed's been really great so far, I love how easy it is to use, and how good the documentation is!

@mankinskin
Copy link

mankinskin commented Nov 9, 2020

Maybe an idea would be providing a function like this:

orders.perform_cmd_with_sink(async move |sink: MessageSink<Msg>| {
   let msg = task1().await;
   sink.send(msg).await?;
   sink.send(task2().await).await?;
   Ok(())
});

Here you have to provide a closure taking a MessageSink<Msg> which implements Sink and maybe returning a Result of some kind? That way you could also handle errors which can occur during sending or during executing some user function.

If that was possible you could send as many messages as you like very easily.

But futures can be very tricky, so I don't know how easy this would be to implement.

@WesleyAC
Copy link
Author

@mankinskin that's a interesting idea! are you thinking that sink.send() is a future that will resolve after the update function is called with the given message? If so, that seems pretty useful in general, as a way of controlling the ordering of execution.

For example, here's something that's hard to express right now (or at least, I can't figure out how to express it nicely):

I have a server that saves notes, as a HashMap<Uuid, String>. I want to have a UI that will display a list of notes, as well as displaying the text of the selected note. Here's a sample model and messages:

struct Model {
    notes: HashMap<Uuid, String>,
    current_note: Option<Uuid>,
}
enum Msg {
    FetchNotes, // uses perform_cmd to fetch notes from the server, and call NotesFetched when the fetch is successful
    NotesFetched(HashMap<Uuid, String>), // set model.notes to the given notes, and re-render the notes list
    OpenNote(Uuid), // Set model.current_note to given Uuid, and render the note text
    NewNote, // send a request to the server to generate a new note, with a server-generated uuid and text
}

Say I want to set model.current_note to the Uuid that is returned by NewNote. I would think to use perform_cmd to call OpenNote after the request is successful, but what I need to do is actually more complicated than that. I need to make the API call to make the new note, then call FetchNotes, and once NotesFetched has finished running, then call OpenNote with the Uuid that I got from the first API call. This is a tricky set of things to do! Maybe there's a more canonical way of doing this kind of thing (I didn't copy the FetchNotes/NotesFetched pattern from anywhere, it's just what made sense to me), but as it stands, doing this sort of thing is kinda hard. I can imagine that API that @mankinskin suggested allowing something like:

orders.perform_cmd_with_sink(async move |sink: MessageSink<Msg>| {
   let uuid = call_new_note_api().await;
   sink.send(Msg::FetchNotes).await?; // after this runs, we know that NotesFetched has been enqueued
   sink.send(Msg::OpenNote(uuid)).await?;
   Ok(())
});

Which would be pretty neat :) But I'm also curious how other people would go about solving this, possibly there's an easy way of getting around this problem that I'm not thinking of.

@mankinskin
Copy link

mankinskin commented Nov 10, 2020

For your use case I think it would be best to just store the uuid in the model and use it to open a note when you receive NotesFetched.

And I think you can actually already send multiple messages with a
msg_sender, but I don't think you can wait for the update function to have completed..

let msg_sender = orders.msg_sender();
orders.perform_cmd(async move {
    let uuid = get_uuid().await;
    msg_sender(Some(Msg::FetchNotes));
    msg_sender(Some(Msg::OpenNote(uuid)));
    None
});

To control execution order and handle message dependencies and such, I think we still have to call the subsequent messages after receiving the awaited message. So basically in your update function, after you receive NotesFetched you can call whatever should happen on OpenNote.

But you are right, it would be much better if we could send messages and await results as you described. That is basically what asynchronous actor frameworks do, but I haven't found one for webassembly.
It would be great if Seed approached this, I could also really use something like this.

@MartinKavik
Copy link
Member

BTW, Seed's been really great so far, I love how easy it is to use, and how good the documentation is!

@WesleyAC I'm glad you like it!

It would be great if perform_cmd had the option of sending multiple messages

I don't see any real reasons to not allow it, however there is a problem:
Stable Rust doesn't allow us to return different types from a function without "hacks" and with a compile-time validation. That's why this monster has been created.
So I would wait until we can rewrite it to a compiler-friendly code and refactor it. We can use @mankinskin suggestion - msg_sender - now as an alternative.

Maybe an idea would be providing a function like this:

...
sink.send(Msg::FetchNotes).await?;
sink.send(Msg::OpenNote(uuid)).await?;
...

I was thinking a bit about Future chaining, but it would be a footgun because WASM still officially supports only a single thread just like JS so it would be too easy to freeze UI. I think we should wait for full multi-threading support and then adapt the whole framework to it.
However if you come with a reasonable idea/API for awaiting/chaining, I won't be against it.
Note: You can read more about WASM multi-threading and alternatives in wasm-mt README.md.

@wkordalski
Copy link

I was thinking a bit about Future chaining, but it would be a footgun because WASM still officially supports only a single thread just like JS so it would be too easy to freeze UI.

Could you give a simple example that would result in UI freeze?
I currently work on a piece of software (it uses Seed) that send messages from futures and can await for update completion (via custom futures).
Single-threaded execution makes the software easier do design, its execution more deterministic (there's no threads that interleave their execution differently each time), and you don't have to think about synchronization of the threads.


Also note that allowing sending multiple messages and awaiting on processing them is equivalent to adding new messages.

enum Msg { A, B, C }

fn update(msg: Msg, ...) {
    match msg {
        Msg::A => orders.perform_cmd_with_sink(async move |sink| {
            X;
            sink.send(Msg::B).await;
            Y;
            sink.send(Msg::C).await;
            Z;
        }
        Msg::B => { P; }
        Msg::C => { Q; }
    }
}

is equivalent to:

enum Msg { A, A1, A2, B { after: Box<Msg> }, C { after: Box<Msg> }}

fn update(msg: Msg, ...) {
    match msg {
        Msg::A => orders.perform_cmd(async move {
            X;
            Msg::B { after: Box::new(Msg::A1) }
        })
        Msg::A1 => orders.perform_cmd(async move {
            Y;
            Msg::C { after: Box::new(Msg::A2) }
        })
        Msg::A2 => orders.perform_cmd(async move {
            Z;
        })
        Msg::B { after } => {
            P;
            orders.send_msg(*after);
        }
        Msg::C { after } => {
            Q;
            orders.send_msg(*after);
        }
    }
}

The most problematic thing is that...

...
sink.send(Msg::FetchNotes).await?;
// ...here we don't have access to model and orders (because futures must be 'static)...
sink.send(Msg::OpenNote(uuid)).await?;
...

...so:

  1. we need to store model in Rc<RefCell<...>> so we can clone it, store and borrow inside futures.
  2. we need to wake futures carefully (so that sending messages doesn't cause BorrowMutError (caused by already borrowed internals of Seed needed during message sending)).
  3. we cannot access orders inside futures, but as for now we cannot do it also

The most helping and quite simple thing that Seed can do (from my perspective), is to allow sending message via msg_sender always. Currently, sending message with msg_sender from inside update function result in BorrowMutError (panic).
Borrowing the queue only for moment of pushing or popping message from the queue would be enough, I think.

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

No branches or pull requests

4 participants