rust-signals icon indicating copy to clipboard operation
rust-signals copied to clipboard

Let `Mutable::set` take Into<A>

Open Billy-Sheppard opened this issue 1 year ago • 4 comments

Hi Pauan,

This PR allows the set method to take Into<A> rather than A. Would you be open to merging this? Is there a (possibly very good) reason not to? Very open to discussion/improvement.

Billy-Sheppard avatar Oct 09 '23 23:10 Billy-Sheppard

What's your use case? The other mutable containers (like Cell) do not accept Into<T>.

Accepting Into<T> would bloat up the file size, because monomorphization will create a new method for each type.

It's not a big deal to just use mutable.set(foo.into()), which doesn't have any monomorphization costs.

Pauan avatar Oct 10 '23 19:10 Pauan

Monomorphization is a good point and a very fair reason to reject this PR.

The use case for me is I find I am often using Mutable<Rc<T>> in my code, which often leads to many into calls.

Billy-Sheppard avatar Oct 10 '23 22:10 Billy-Sheppard

Could you explain more about how you're doing the wrapping?

If you're using dominator, then the standard style is for components to have a fn new() -> Arc<Self> function, so you don't need to call .into() on that.

So the only time you need to manually call .into() is with Mutable<Arc<str>>

Pauan avatar Oct 10 '23 22:10 Pauan

As an example I have a Model for a page which contains fields of Mutables. If the type is non-copy then I wrap the T in an Rc.

struct Model {
    copy_type: Mutable<T>,
    non_copy_type: Mutable<Rc<T>>,
    ..
}

Billy-Sheppard avatar Oct 10 '23 22:10 Billy-Sheppard