Additional tests for #613

[NullVoxPopuli]

While debugging #613 (comment)
I could no longer reproduce the issue.

So... I wonder if there is something specific about that reproduction repo in the comment that is causing problems?
I've kept the tests, because having assurance that certain behaviors work is nice.

[apellerano-pw]

I think the situation only arises if you tear down the rendered DOM while the conditional is still true. The tests here set the conditional back to false, which is why the destructor has a chance to run.

[NullVoxPopuli]

ah ok! thanks -- lemme try that back on the GlimmerVM side of things: glimmerjs/glimmer-vm#1411

[NullVoxPopuli]

I've reproduced the issue in glimmer-vm.

Now here comes philosophy:

• if the element doesn't exist anymore, is there anything to clean up?

[apellerano-pw]

Our team has adopted modifiers pretty quickly, I think because they fill the gap left by Octane when removing didInsertElement / willRemoveElement. The target for our modifiers is sometimes the element, but often not. Take the on-click-outside modifier demonstrated in the Ember Guides which modifies document instead of element. It's disastrous if the destructor never runs, amounting to a leak. The more you instantiate the modifier the more click listeners end up weighing down document! We use that modifier, and have another similar modifier for adding keyboard shortcuts to the app.

These may in time be considered abuses of modifiers, since they produce side-effects from a certain philosophical perspective. But they're a good way to do DOM lifecycle events in Octane and even encouraged to be used this way by the docs. If Ember were to take a philosophical stance against usages like this I think it would leave a functionality hole in the framework and contradict the docs.

[NullVoxPopuli]

I 100% hear you -- and the need for the ability to add listeners to the document is essential.
There are def alternate ways to implement the functionality you want, but lemme poke around see if I can confirm this is an oversight with modifier behavior, since it's specific the conditional application of the modifier, I have a hunch: yes -- it's an oversight)