Adds a widget destroy method

[saqimtiaz]

This PR adds a widget destroy method, and builds on the earlier work in #7468, which was reverted to allow more time for performance testing.

The key difference to that implementation is that a widget only removes its DOM nodes if an ancestor widget is not already going to remove an ancestor DOM node.

Apologies for the extended delay on this, it slipped through the cracks until I started reviewing all local branches that had not been pushed. It would be nice to get this into the upcoming v5.4.0 release that also allows for the possibility of limited backwards compatibility breaks.

To do:

• performance testing vs implemented destroy() function #7468
• peer review to compare to implemented destroy() function #7468 and add depth first destroy method  #7233

[github-actions]

Confirmed: saqimtiaz has already signed the Contributor License Agreement (see contributing.md)

[github-actions]

📊 Build Size Comparison: empty.html

Branch	Size
Base (master)	2528.0 KB
PR	2528.7 KB

Diff: ⬆️ Increase: +0.7 KB

[netlify]

✅ Deploy Preview for tiddlywiki-previews ready!

Name	Link
🔨 Latest commit	5357eb9
🔍 Latest deploy log	https://app.netlify.com/projects/tiddlywiki-previews/deploys/6851273bf4759f0008d1a789
😎 Deploy Preview	https://deploy-preview-9097--tiddlywiki-previews.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[saqimtiaz]

@buggyj could you please check if this passes your tests? I tried to run them myself but it was unclear as to what a successful outcome looks like.

[pmario]

I did run https://destroytest.tiddlyhost.com/ with Saq's changes here and there seems to be some differences.

@buggyj -- I did test it with your destroytest at tiddlyhost according to your information in the test tiddlers. I think I did understand, what they should do. Using this branch with your plugin there is a different behaviour.

[saqimtiaz]

I had another look at those tests and I am not certain they are universal enough to be relevant here.

For instance, they expect that the DOM node created by a widget persists until after the base class destroy() method has returned. However, the intention with the proposed code is that you do any additional tear down before calling the base class destroy() method.

[buggyj]

@buggyj could you please check if this passes your tests? I tried to run them myself but it was unclear as to what a successful outcome looks like.

The test for v5.3.0 are here https://destroytests530prerelease.tiddlyhost.com/ , I will try them with this branch.

[buggyj]

@buggyj could you please check if this passes your tests? I tried to run them myself but it was unclear as to what a successful outcome looks like.

@pmario @saqimtiaz I put the tests here https://destroytests537prerelease.tiddlyhost.com/ , they all pass.

These are only basic tests, I have not thought about DOM MutationObserver I believe that @linonetwo uses these...

[saqimtiaz]

@buggyj thank you

@linonetwo when you get the chance please have a look.

[linonetwo]

@saqimtiaz Great, please merge it soon.

[linonetwo]

@saqimtiaz Could you add some test for this? Like a custom widget and test it is called. And dom tree only removed once. Or something related to performance.