fix: ensure everything is disposed of consistently

[Giovanni-Tably]

Hi there!

This one popped up for us as a BorrowMut error on the on_cleanups secondary map, here's what I think was going on:

• dispose_node was removing nodeA from the reactive graph, but not running the cleanups.
• The slot in the slotmap was reused by nodeB, and nodeB used on_cleanup.
• The old cleanups were still there, so, when inserting the new cleanup, the old ones were dropped since the slot is the same but has a higher version.
• The old cleanups contained Drop impls that triggered some work, which were triggered inside slotmap.
• If any of the triggered work used on_cleanup, we recurse and panic.

With this patch I tried to simplify and make it more obvious what cleaning up a node and disposing of it means, hopefully my understanding of what's going on here is correct.

The commits are meant to be easy to audit individually, and refactor commits are meant to match the original behaviour exactly.

And as always, thank you for leptos, it has been great!

[Giovanni-Tably]

Besides this patch, an interesting option might be to wrap things like the on_cleanups in a newtype that panics (or emits a warning to report the issue) if dropped unintentionally to catch subtle bugs as early as possible.

[Giovanni-Tably]

After auditing the SecondaryMaps, it seems like node_owners and node_properties (of the owner) are also not cleaned up.
I have this commit (Giovanni-Tably@bb57d47) that should maybe fix it, but I don't fully get what the difference between remove_scope_property and dispose_node was meant to be and the interaction with multiple runtimes.

Edit: also contexts. And tbc node_owners and node_properties do not contain user-provided values, so the Drop issue doesn't apply.

[gbj]

It would be very helpful to have a reproducible example of the issue(s) this is addressing, either here or in a linked issue.

[Giovanni-Tably]

Of course, I added a couple of regression tests which currently fail on main.

Something not covered is the fact that subscribers are now also cleaned if the node is disposed of directly (previously only if they were a child of a cleaned up node).

---

Separately, I think node_sources may be leaking. Specifically: if a memo runs twice and subscribes to different signals, all of them are kept in node_sources. I think this happens even if the signals are disposed of.
Looking at the code though, I can't tell if it was intentional. The subscribers of the sources are cleaned. Is that because when the signals are mostly the same it's faster?

[gbj]

Thanks very much! The tests are great and will be helpful to make sure this correct behavior carries over into the next version, too.