Skip to content

Conversation

Billy-Sheppard
Copy link

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.

@Pauan
Copy link
Owner

Pauan commented Oct 10, 2023

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.

@Billy-Sheppard
Copy link
Author

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.

@Pauan
Copy link
Owner

Pauan commented Oct 10, 2023

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>>

@Billy-Sheppard
Copy link
Author

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>>,
    ..
}

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

Successfully merging this pull request may close these issues.

2 participants