Enterprise-ng: Memory leak on soho-toolbar.component

[GMInf]

'.more' - 'mouseover' event was not unsubscribed on ngOnDestroy() causing a memory leak on the soho-toolbar component.

To Reproduce

1. Dynamically create a '\'
2. Remove the '\'
3. Repeat several times
4. On developer tools, check Heap Snapshot, all instances of soho-toolbar is still retained.

Expected behavior
soho-toolbar should be freed from memory after use. 'mouseover' event should be unsubscribed on ngOnDestroy().

Version

• ids-enterprise-ng: 5.4.2

Screenshots
'mouseover' event was not removed:

Platform

• Device: PC
• OS Version: Windows 10
• Browser Name chrome
• Browser Version

[tmcconechy]

This issue may be fixed by addresing https://github.com/infor-design/enterprise/issues/2496

[tmcconechy]

Assigning @EdwardCoyle to see if we can address this when solving https://github.com/infor-design/enterprise/issues/2496

[pwpatton]

We need this one in 5.5.x

[pwpatton]

@GMInf nice find! We've been looking for memory leaks so it's nice to find another we can squash.

[tmcconechy]

@pwpatton he's done a PR but i dont know what he needs to do to get it over here https://github.com/GMInf/enterprise-ng/pull/1 do you want to just recreate the PR here on a branch for @GMInf ?

[pwpatton]

@tmcconechy We'd need that PR against 5.5.x

[pwpatton]

This is the change I'm about to PR.

  ngOnDestroy() {
    // call outside the angular zone so change detection isn't triggered by the soho component.
    this.ngZone.runOutsideAngular(() => {
      if (this.jQueryElement) {
        this.jQueryElement.off();
        this.jQueryElement.find('.more').off();
      }
      if (this.toolbar) {
        this.toolbar.destroy();
        this.toolbar = null;
      }
    });
  }

[pwpatton]

I'll PR and include @GMInf as an approver

[pwpatton]

@tmcconechy yea, this fixes only the soho-toobar memory leak and the ep/toolbar still seems to be leaking memory.

[tmcconechy]

OK waiting for your PR to approve. Thanks!

[GMInf]

Hi! We are currently using ids-enterprise-ng 5.4.2. Will this fix be included in that version too? Or do we need to upgrade to 5.5 to get the fix? Thanks!

[tmcconechy]

@GMInf

You would need 5.5 for the fix. We don't normally backport fixes to old versions unless it is a blocker.

[CindyMercadoReyes]

Moving this ticket to the Done folder. There is no way for QA to test this ticket.