fix(cds-icon): fix globalstateservice memory leak

ensure the component is connected before subscribing to the globalstateservice Fixes #61 Signed-off-by: Ashley Ryan <asryan@vmware.com>
vmware-clarity · May 26, 2022 · df13775 · df13775
1 parent d43375d
commit df13775
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 7 deletions.
diff --git a/projects/core/src/icon/icon.element.spec.ts b/projects/core/src/icon/icon.element.spec.ts
@@ -8,6 +8,7 @@ import { html } from 'lit';
 import '@cds/core/icon/register.js';
 import { CdsIcon } from '@cds/core/icon/icon.element.js';
 import { ClarityIcons } from '@cds/core/icon/icon.service.js';
+import { GlobalStateService } from '@cds/core/internal';
 import { componentIsStable, createTestElement, removeTestElement } from '@cds/core/test';
 import { renderIcon } from './icon.renderer.js';
 
@@ -94,6 +95,31 @@ describe('icon element', () => {
       expect(component.requestUpdate).not.toHaveBeenCalled();
       expect(component.getAttribute('shape')).toEqual(testShape);
     });
+
+    it('does not subscribe to the global state handler after the element is disconnected', async () => {
+      const spy = spyOn(GlobalStateService.stateUpdates, 'subscribe');
+
+      testElement = await createTestElement(html`<cds-icon shape="testing"></cds-icon>`);
+      expect(GlobalStateService.stateUpdates.subscribe).toHaveBeenCalledTimes(1);
+
+      spy.calls.reset();
+
+      // https://github.com/vmware-clarity/core/issues/61
+      // verify the element is not subscribed when it is removed before connecting
+      const div = document.createElement('div');
+      document.body.appendChild(div);
+
+      const icon = document.createElement('cds-icon');
+      icon.setAttribute('shape', 'testing');
+      div.appendChild(icon);
+      icon.remove();
+
+      await new Promise(resolve => setTimeout(resolve, 0));
+
+      expect(GlobalStateService.stateUpdates.subscribe).not.toHaveBeenCalled();
+
+      div.remove();
+    });
   });
 
   describe('size: ', () => {

diff --git a/projects/core/src/icon/icon.element.ts b/projects/core/src/icon/icon.element.ts
@@ -169,13 +169,15 @@ export class CdsIcon extends LitElement {
   firstUpdated(props: PropertyValues<this>) {
     super.firstUpdated(props);
 
-    let prior = 'unknown';
-    this.subscription = GlobalStateService.stateUpdates.subscribe(update => {
-      if (update.key === 'iconRegistry' && ClarityIcons.registry[this.shape] && prior !== this.shape) {
-        prior = this.shape;
-        this.requestUpdate('shape');
-      }
-    });
+    if (this.isConnected) {
+      let prior = 'unknown';
+      this.subscription = GlobalStateService.stateUpdates.subscribe(update => {
+        if (update.key === 'iconRegistry' && ClarityIcons.registry[this.shape] && prior !== this.shape) {
+          prior = this.shape;
+          this.requestUpdate('shape');
+        }
+      });
+    }
   }
 
   disconnectedCallback() {