onDestroy not called in controller if node is removed and wrapped by other DOM nodes

[stonecooler]

Expected Behavior

Callback "onDestroy" should be called in the controller of the node if node is removed from DOM and if the removed node is wrapped by other nodes (not controlled by excellent.js) like in this structure:

<span>
   <i e-bind="lifeSpan2">lifeSpan2</i>
</span>

Actual Behavior

Callback "onDestory" will not be called. I debugged the problem and it seems that the MutationObserver Callback is not fired. In every browser!

Steps to Reproduce

See http://plnkr.co/edit/JO6pU7RZWpwH1eVX or: http://plnkr.co/edit/1iFoMloB5hHpjufe?preview

Environment

In Chrome, Firefox + Edge.

Quickfix I found:

this.watch = function (e) {
  if (mo) {
      mo.observe(e, {childList: true, subtree: true});
  }
};

or

this.watch = function (e) {
  if (mo) {
      mo.observe(e.parentNode, {childList: true});
  }
};

...it works, but it makes no sense to me at the moment. I will try to read more about the MutationObserver.

[vitaly-t]

Cheers! See my comments against your PR ;)

[stonecooler]

Hello Vitaly, I removed the "console" logging from my forked/PR code and I also tried writing tests, which does not work for this issue, because the issue is caused by the MutaionObserver which is not supported in Jest (as you also mention in you source code).

What is your opinion: Is a test with a headless browser engine with JEST (and Travis CI) possible? Should we do that?

Wanted to ask you first before I go any deeper.

Thanks.

[vitaly-t]

@stonecooler

Is this the only change that you are suggesting?

I do not like the other ones, because method findOne is supposed to throw an error when anything other than one controller is found, it is its only purpose, as documented.

---

i.e. if just adding subtree: true option is sufficient, then we can do this change. I wouldn't change the other things though.

[stonecooler]

@vitaly-t

i.e. if just adding subtree: true option is sufficient, then we can do this change.

Not quite. You have to add also manualCheck() in function mutantCB see here.

I wouldn't change the other things though.

Okay for me. I understand it. It's your Lib. I rolled it back also in my fork.

[vitaly-t]

I have made those changes, and for now published it as a beta - 1.8.0-beta.0:

npm install excellent.js@1.8.0-beta.0

Please test it well on your side, and if everything works well, then I will make it an official release.

[vitaly-t]

@stonecooler It's been over one week since I gave you the updated version. Any chance you can get back to me on this?

[stonecooler]

@vitaly-t Sorry I have not been in touch for so long. Yes the 1.8.0-beta.0 works for me. Thank you.