Ownership of virtual_dom_rs::patch params about percy HOT 7 CLOSED

Yes, a regular reference will probably suffice. The good thing is that the compiler will tell us whether it's needed or not 🙂

from percy.

Ahhh thanks for raising this.. I'd actually forgotten about this.

It's not mandatory, IIRC I had a lifetime issue and just did it this way to make it work. This was either before web_sys existed or shortly after it was published..

Regardless it's not desired.. so if you want to take a stab at making it mutable I'd be happy to answer any questions along the way!

from percy.

Will probably give this a try next week!

from percy.

Sweet! Again, I really don’t remember the problem, it might not have even been lifetimes and instead have been multiple mutable references or something.

Regardless I’m sure it was just poor form on my end and not a fundamental issue.

Although does this suggest that we might want a reference instead of a miracle reference ...?

rustwasm/wasm-bindgen#1061

from percy.

Hm, it's not that easy, because you are inserting the element into the nodes_to_patch hashmap.

This should be doable with only references by rewriting the patching algorithm. I'll try to take a quick look when I find time, but don't know yet if that'll be today or not.

from percy.

Ok, changing it to references using the same recursive algorithm is a bit tricky because of the lifetimes involved.

Since the Element is clonable we could simply clone it (see master...dbrgn:patch-ownership), but I'm not sure if that's worth the improved API because this way the element always gets cloned, while it might not need to be cloned if the user doesn't need the reference to the element anymore after patching.

Another option would be to accept the element by value, to create an Rc from it, and to pass that Rc to the find_nodes function. Then in the end we could return the original element by value. But every Rc causes an allocation and I'm not even sure how to unwrap the Rc to recover the original value 🙂 So I think it might be best to stick to the current approach unless someone figures out lifetime annotations to be able to stick to references only.

TLDR: The user can clone the element before passing it to the patch function.

from percy.

SWEET thanks for looking into this!

Ah so it was a lifetime issue - I vaguely remember haha ..

Sweet this sounds great for now 👍🏿

from percy.