Dispose layer tree node event listeners.

[bentrm]

I think there are some dangling event listeners associated with the layer node. I hope to fix this with this PR.

As far as I can tell, there is no reasonable way to hook into the destruction of the LayerTreeModel of the LayerNode to fix this so I used the maps 'removelayer' event. 'preremovelayer' is not an option as other listeners outside of this scope may cancel removing the layer. That's also the reason for caching a reference to the actual map. When 'removelayer' is called the layers map property is already lost.

[marcjansen]

Any chance you can add some tests? On first glance this looks fine. Also, is #358 a replacement?

[bentrm]

There are no explicit tests for GeoExt.tree.LayerNode right now so I was a bit hesitant and deferred it. I'm happy to help but will not have time to look into this before the weekend.

[marcjansen]

Please also use curly braces around one liners.

[marcjansen]

As far as I can tell, there is no reasonable way to hook into the destruction of the LayerTreeModel of the LayerNode to fix this …

Have you tried (untested):

// …
init: function(target) {
    this.target = target;
    this.target.addListener('beforedestroy', this.cleanupMethod, this);
    // …
}
// …

Except for this and two other comments this is good to go.

[bentrm]

Yes, I tried that. this.target is an instance of GeoExt.data.LayerTreeModel which is an Ext.data.Model. The only event that is advertised seems to be idchanged. Furthermore, I tried to destroy the LayerNode plugin by hooking into the destroy method of the LayerTreeModel but it is never called as "destroy" of Ext.data.Model is used to delete the record which seems not to be the case here.
I think the proposed solution is borderline hackish but I'm out of ideas.

[marcjansen]

I will give my OK for a merge if we add a big TODO somewhere and maybe even an issue as the approach seems more than hackish to me.

Nonetheless the result is better than what we had before, so plus one for merging this once it is updated with a big TODO.

Can you update this @bentrm?

[bentrm]

as the approach seems more than hackish to me.

I agree, absolutely. Will add a TODO.

[chrismayer]

Hi guys,
any news on this? thanks!

[bentrm]

Promise to take care of this this afternoon. Sorry about that, I know you want to ship on 4th.

[bentrm]

I amended the last commit to include TODOs hope you're ok with that. Thanks for your patience.

[marcjansen]

👍 thanks!