Relax the Clone bound on Message types

[abreis]

I've found that the Clone bound on Message types can be a hit to ergonomics, particularly when working with Commands.

Example: a typical fallible async function returns some form of Result<_, Error>. The standard approach to call such a function in iced seems to be:

Command::perform(async_method(), Message::VariantThatWrapsOutput)

and then handle the message in the view's update().

However, most error types are not Clone (e.g., std::io::Error), so that future's output can't be wrapped in a Message directly and some mapping of the error to a clonable type is needed.

I've been going through the code in iced_native and from what I can see the Clone bound is only required by TextInput and Button, for code such as:

Event::Keyboard(keyboard::Event::Input {/* (...) */}) 
if self.state.is_focused => match key_code {
    keyboard::KeyCode::Enter => {
        if let Some(on_submit) = self.on_submit.clone() {
            messages.push(on_submit);
        }
    }

From what I understand the aim of this is to produce a new message on each key press. This PR proposes changing these fields from:

    on_submit: Option<Message>,

to

    on_submit: Option<Box<dyn Fn() -> Message>>,

i.e., lazily producing the Message variant. This allows us to get rid of the Clone bound.

I've opened this as a draft to hear your thoughts first. I haven't adjusted documentation nor the web versions of TextInput/Button. What do you think?

[Songtronix]

Related: #191 #155

[abreis]

Ah crap, didn't see those. Thanks.

[abreis]

@hecrj

I see that your opinion on this is that the user should nest cloneable messages in their own types, so I'll close the PR.

Still, I'd like this to be one more indication that perhaps the Clone bound is not user-friendly, and that nesting is not an obvious solution.

[hecrj]

@abreis

I've been going through the code in iced_native and from what I can see the Clone bound is only required by TextInput and Button

I believe the implementation details and needs of a particular part of the codebase shouldn't get to decide what a shared concept of the architecture is or isn't.

In particular, it seems you are proposing a Message type should never (even in the future) have a Clone bound because we can currently avoid it with a closure in TextInput and Button.

The main issue here is that the statement "a message should never be cloneable" is completely against the event sourcing paradigm of The Elm Architecture, which iced is heavily inspired by.

In event sourcing, all messages (or events) are made of pure immutable data. As a consequence, they can be freely reused, copied, stored, serialized, and deserialized. Moreover, the current state of an Application can be deterministically derived from a log of events (the source of truth) by simply initializing its state and running update for each one of the events.

Thanks to this, Elm has an awesome time-travelling debugger with import/export features.

However, most error types are not Clone (e.g., std::io::Error), so that future's output can't be wrapped in a Message directly

If a type isn't Clone, then it means it's not made of pure immutable data, therefore it shouldn't be wrapped in a message.

some mapping of the error to a clonable type is needed

I believe this is a case where the architecture of the library forces you to improve the boundaries between the different parts of your codebase.

It is much better to deal with any library errors right where they happen (the async call) and map them to meaningful application errors, instead of polluting your update logic with a lot of different libraries and domain-specific logic.

If we want to perform a login request with reqwest, we should not wrap a reqwest::Error in a message and check the status code in update. Instead, it is much better to deal with error mapping inside the Future and return our own LoginError type.

I see that your opinion on this is that the user should nest cloneable messages in their own types, so I'll close the PR.

My opinion is that all messages should be able to implement Clone if necessary. The Clone boundary will be required by an Application trait when we end up relying on event sourcing.

[abreis]

@hecrj
Understood, and thank you for the detailed explanation.

I feel that the disconnect, and part of the reason why you've been seeing a few people complain about the Clone bound in specific, is that (at least for me personally) in Rust something that is Clone often carries an implicit meaning that it would make logical sense for multiple copies of that particular object to exist.

The reverse is also true, then: when I create a Message in iced, I don't expect the message to arrive twice, for example. In my mental model that message is unique and should pop up at an update() sometime in the future, and should also show up there as owned, not borrowed. So when the framework asks for the message to be Clone, I begin to wonder: why is this message going to be duplicated? Hence the reaction.

(to be clear, I'm not trying to advocate for a !Clone bound any more, your explanation made perfect sense)

If we want to perform a login request with reqwest, do not put a reqwest::Error in update and check the status code there. Instead, it is much better to deal with error mapping inside the Future and return your own LoginError type.

These are important insights for newcomers, along with the patterns of splitting the Message type, and the whole concept of composability in general in iced. I hope one day the project can gain some book-style documentation that points new users to the right way of structuring their apps.

That said, I've been really enjoying working with iced. Thanks!

[hecrj]

in Rust something that is Clone often carries an implicit meaning that it would make logical sense for multiple copies of that particular object to exist

Yes, but this is completely applicable to messages. For instance, a bunch of ButtonPressed messages can exist at the same time. I fail to see where the disconnect is.

So when the framework asks for the message to be Clone, I begin to wonder: why is this message going to be duplicated?

Wouldn't you ask the same question if the API asked for a closure with no arguments? It'd personally strike me as even more odd.

The Clone is there because the Message may be indeed duplicated. Depending on the runtime implementation, there is a chance you may press a Button twice between view calls.

In my mental model that message is unique

I guess that's the issue. There is nothing unique about a message. When you press the same button twice, the same message is produced twice, just as if it were cloned.

In fact, there should be no way for you to know where the message is even coming from.

[hecrj]

These are important insights for newcomers, along with the patterns of splitting the Message type, and the whole concept of composability in general in iced. I hope one day the project can gain some book-style documentation that points new users to the right way of structuring their apps.

I have plans to start building an open-source application with iced soon. It will be complex and big enough to serve as a real example of how to use the library and showcase many different patterns that work well with it.

[abreis]

I was referring to individual instances of a type, not e.g. an enum variant in general. Meaning: if I press a button twice, two messages are created, each are unique (though they are of the same type/variant), and two messages arrive. If that type isn't Clone, I can trust that the framework won't turn those two messages into four, for whatever reason.

You're right about the closure with no arguments producing the same message over and over, though note that the closure could move other objects inside it. In a fictional scenario a closure could return a different message every time it was called.

But again, this is besides the point. I understood the Elm approach and how it maps to Rust here.

I have plans to start building an open-source application with iced soon. It will be complex and big enough to serve as a real example of how to use the library and showcase many different patterns that work well with it.

Look forward to seeing it!

[hecrj]

if I press a button twice, two messages are created, each are unique

How are they unique if they are not different? You could say they are different instances in memory, but that distinction is hardly useful when dealing with messages.

Imagine you used u32 as your Message type, would you still care about this? Is a 3 different from another 3? Enum variants of pure, immutable data are not different.

If that type isn't Clone, I can trust that the framework won't turn those two messages into four, for whatever reason.

Why is this important when messages are simple events?

The closure approach doesn't guarantee it will produce only two messages either. You still have to trust the framework to call your closure only when the button is pressed, same as Clone.

Even if we removed the Clone boundary, a runtime could call view between button clicks just to produce more messages and feed those to update.

What kind of trust are you referring to?

In a fictional scenario a closure could return a different message every time it was called

Only an FnMut would be able to do that, and using one would break the purity of view in The Elm Architecture.

[abreis]

Only an FnMut would be able to do that, and using one would break the purity of view in The Elm Architecture.

No need for FnMut 🙂 for example:

#[derive(Debug)]
enum Message {
        TypeOne,
        TypeTwo,
}

fn some_framework_method<F: Fn() -> Message>(closure: F) {
    println!("{:?}", (closure)() );
}

fn main() {
    some_framework_method(|| {
       let now = std::time::SystemTime::now();
       
       if now.elapsed().unwrap().as_secs().is_power_of_two() {
           Message::TypeOne
       } else {
           Message::TypeTwo
       }
    });
}

• This is obviously contrived, and certainly not in the spirit of Elm.
• You are correct that I have to trust the framework to do the right thing and not call the closure multiple times. And I already trust the framework to not send me more messages than the times I press a button (nor fewer).
• Not being Clone is simply a contract, enforced by Rust itself, that what I give you won't be duplicated (not in safe Rust anyway). Hence the mental model thing.

[hecrj]

@abreis Sure, Rust isn't pure. You could also use a RefCell if you wanted to. There is no way for the library to completely protect you from shooting yourself in the foot.

The whole point of the library is to guide you towards better practices.

Not being Clone is simply a contract, enforced by Rust itself, that what I give you won't be duplicated (not in safe Rust anyway). Hence the mental model thing.

I understand. But I still fail to see why would you ever care that a Message won't be duplicated, unless it contains impure data, and particularly in the scenarios we discussed.

Additionally, Clone is a very common trait and it should be implemented whenever possible.

[hecrj]

I am guessing part of the issue is that the reason of the Clone bound for a Button is not apparent, so it may lead users to think the runtime may send multiple messages on press.

However, I believe you are the first person to bring this up. Most of the discussions around Clone have been centered around impure data. This also seems to be what prompted you to open this PR:

However, most error types are not Clone (e.g., std::io::Error), so that future's output can't be wrapped in a Message directly and some mapping of the error to a clonable type is needed.

[snoopdouglas]

@hecrj I'm new to Iced and just ran into this exact issue, went to check the type of iced::Application::Message, and saw that it was Debug + Send so, yeah, I was pretty confused at first.

The reason I haven't used Clone on my Message enum is indeed because of Results.