maciejhirsz / kobold

Easy declarative web interfaces.
https://docs.rs/kobold/
Mozilla Public License 2.0
385 stars 7 forks source link

Flatten `stateful` allocations. #63

Closed maciejhirsz closed 1 year ago

maciejhirsz commented 1 year ago

Previously the StatefulProduct was an Rc that contained Box<dyn Product<S>>:

https://github.com/maciejhirsz/kobold/blob/9024bf7c6c024e60bfd164f50bc0b905c5b6e6d9/crates/kobold/src/stateful.rs#L31-L34

This PR manages to cram the dyn Product<S> flat into the Rc:

https://github.com/maciejhirsz/kobold/blob/f8af3a8abb8a01b1ca7a5c167d4db59ae9fdd7e4/crates/kobold/src/stateful.rs#L31-L35

I've also removed the Hook::signal method in favor of Hook::bind_signal, as trying to create a Signal inside a view was a footgoon. Hook::bind_async that automatically spawns a future as a task might be an even better solution here, and could be made to interact with the bind! macro.

:warning: Safety TODO

Previous version relied on Rc::new_cyclic to create a weak reference to the product, which was then needed to be upgraded to a strong reference before changes could be made. This is no longer the case and while it's safe under normal circumstances, it's technically possible to call a bound event callback from within a view with a synthetic Event and cause UB as it interacts with uninitialized memory.

Possible solutions (not exclusive, we can have both):