doesn't work?

[mindplay-dk]

@JorgeBucaran Was hoping to re-apply my changes to your branch, but... I noticed your test-case doesn't seem to be testing anything: you have a var removed, which you initialize as false - but that var is never changed, and the only assertion is to see if that var is false, so...

I replaced your test-case with the one from my branch, and it fails: it doesn't look like ondestroy is ever actually called? I can't figure out why.

I noticed one difference between your and my implementation, is you pass the about-to-be-destroyed HTML element to the handler - whereas my implementation passes the VNode instance. What I figured is, the element will certainly be destroyed at this point, so what use is it to a callback? Instead, I passed the VNode, which might contain other props or handlers that might be of interest - but if you wanted the actual element, you should be using onremove, I think?

In fact, I think I'd prefer to call ondestroy after the element has been removed, which would be consistent with onupdate which fires after the update, and oncreate which fires after creation.

If the only other thing you did was reorder the functions (it's really hard to tell from the diff) perhaps consider working from my branch instead? So that we get a clean commit with just the reordering - which shouldn't take long to do.

[jorgebucaran]

@mindplay-dk I'll run your test on my impl and get to the bottom of this! 😄

[jorgebucaran]

@mindplay-dk I see what's going on. What happened is that I changed how onremove works on that PR.

Before you would return a function to get the remove function, now you get it as the second argument to the handler and you need to explicitly call it.

I changed your test case to call this remove() directly and it works as it should, the actual test still fails, but that's because of a bug, ondestroy is called deep-first inside out so "removed b, destroyed c, destroyed b".