Prevent firing callbacks until members are destroyed

[jankapunkt]

I found a few errors related to #213 (cc @lynchem) when I moved to a new route and a new template is drawn.

I debugged my Templates and found that child templates of the page I "left" were not destroyed, while the page main template was destroyed correctly and the child template was also in _domrange.members.

So I started a deeper debugging session and found that Blaze._fireCallbacks(view, 'destroyed') was placed before view._domrange.destroyMembers and prevented the members from being removed, because the whole method just aborted after _fireCallbacks was called.

Placing it after them removes the members and the errors are gone, while at least our app tests are running fine.

I can't say if this is related to the errors in #213 but I can say that this was definitely a memory leak.
I also don't know where I should place tests for this to prevent future regressions (especially if we do a lot of rewrites for 3.0). Please help whoever can.

[StorytellerCZ]

LGTM

Blaze 2.6.1?

[jankapunkt]

I don't know, I tested for our app but I have no comparison or other setups that might break. Maybe we get some more people to test, especially those, having trouble with #213

[StorytellerCZ]

Let's give them some time then. I would like to see it in 2.6.1 if there are no complains.

[StorytellerCZ]

@jankapunkt what should be the line for changelog?

[jankapunkt]

@StorytellerCZ

• fix destroying child templates when parent is destroyed; prevents memory leak and DOMRange "not attached" error

[lynchem]

That's great @jankapunkt , good detective work. We've never found a way of reproducing #213 so can't really help with tests. We can give it a quick test in staging and if all is order launch it. We'll know after a couple of days if it's worked as it fires pretty much daily.

And the removal of a core memory leak like this is also great news 👏🏻
When do you expect to launch it?

[jankapunkt]

From my pov we could do an RC release asap which is great for testing but that's not my call. @fredmaiaarantes what do you think?

[fredmaiaarantes]

From my pov we could do an RC release asap which is great for testing but that's not my call. @fredmaiaarantes what do you think?

I also think it's worth it.

cc @denihs

[denihs]

I'm going to merge this to the branch release-2.6.1 to centralize everything there. #371

[denihs]

Guys, blaze@2.6.1-rc.0 is ready to be tested. #371

[jankapunkt]

Awesome! cc @lynchem I hope it will fix your issue as well. For the memory leak I will create a small test project for profiling next week.