Clear Data for Removed Widgets

[MrGVSV]

Problem

Currently, when a widget is removed, it leaves behind pretty much all data associated with it. This is because we typically store that data based on its Index, which can continue to exist after the widget is removed.

There should be a way for this data to be cleaned up. Doing so will also have the added benefit of granting the option of responding to the removal (such as for cleanup functions described in #45) via a Drop impl (presumably).

Possible Solutions

So far I can only think of these two suggestions (but there's probably a better way to do it):

1. Using diffing (not sure if #1 has been resolved or not), we could get a list of all the Change::Deleted nodes. Then we just iterate over them and manually remove the necessary data from KayakContext. This method is probably slower and more cumbersome, but probably much simpler to implement with adequate diffing.
2. Utilize a crate like weak_table, to create the data HashMap and HashSet fields. Then add a trait method to Widget allowing it to return a reference to its Index (or maybe a custom wrapper struct). This way we could use a Weak<Index> as the key to automatically drop the stored data when the widget is removed. This way is a bit cleaner, but adds a dependency and requires changing a lot more code to integrate it. (Just did some quick testing and I guess widgets are dropped every render? I assume when rebuilding the tree? So maybe this whole idea wouldn't work anyways...)

Not sure how to best approach this 🤔

[StarArawn]

Using diffing (not sure if Re-enable Diffing #1 has been resolved or not), we could get a list of all the Change::Deleted nodes. Then we just iterate over them and manually remove the necessary data from KayakContext. This method is probably slower and more cumbersome, but probably much simpler to implement with adequate diffing.

Diffing isn't fixed yet. We need to wrap props in Binding<Prop> inside the widget proc macros. Once that's done we can ignore updating children. No need to diff either since the notify on the Binding does that for us. Once I'm back to working on kayak again I'll probably pick this task up first.

Utilize a crate like weak_table, to create the data HashMap and HashSet fields. Then add a trait method to Widget allowing it to return a reference to its Index (or maybe a custom wrapper struct). This way we could use a Weak<Index> as the key to automatically drop the stored data when the widget is removed. This way is a bit cleaner, but adds a dependency and requires changing a lot more code to integrate it. (Just did some quick testing and I guess widgets are dropped every render? I assume when rebuilding the tree? So maybe this whole idea wouldn't work anyways...)

We can use Change::Deleted with an iterator. We'll already have a Index to the widget there that we can use to delete all of its relevant data. Currently widget's are removed only from the tree.

[MrGVSV]

Diffing isn't fixed yet. We need to wrap props in Binding<Prop> inside the widget proc macros. Once that's done we can ignore updating children. No need to diff either since the notify on the Binding does that for us. Once I'm back to working on kayak again I'll probably pick this task up first.

Sounds good. And, yeah, just wanted to mark down the issue. There's no rush to come back 🙂

We can use Change::Deleted with an iterator. We'll already have a Index to the widget there that we can use to delete all of its relevant data. Currently widget's are removed only from the tree.

Yeah that's what I figured. That should be a fine solution, we'll just have to make sure we properly remove the data when they should be removed.

Also just a good thing to know in general haha. So we keep the built widgets in current_widgets so that we don't have to re-build them, right?

[StarArawn]

Also just a good thing to know in general haha. So we keep the built widgets in current_widgets so that we don't have to re-build them, right?

Yep

[StarArawn]

Fixed with the rewrite! 👍 Please reopen if you feel as though this is still an issue.