Merge pull request #2506 from exadel-inc/hotfix/mixin-livecycle-disco…

…nnect

fix(esl-mixin-element): major fix for nested hierarchy mixin disconnection

src/modules/esl-mixin-element/test/mixin.prototype.test.ts

@@ -0,0 +1,76 @@
+import {ESLMixinElement} from '../ui/esl-mixin-element';
+
+describe('ESLMixinElement.prototype implements all required ESLComponent API methods', () => {
+  class CTestMixin extends ESLMixinElement {
+    static override is = 'c-test';
+  }
+
+  let $el: CTestMixin;
+
+  const $host = document.createElement('div');
+  $host.toggleAttribute(CTestMixin.is, true);
+
+  beforeAll(async () => {
+    document.body.appendChild($host);
+    CTestMixin.register();
+
+    await Promise.resolve();
+    $el = CTestMixin.get($host) as CTestMixin;
+  });
+
+  describe('ESLMixinElement.prototype.$$attr persist and correct', () => {
+    const attrName = 'test-attr';
+    const attrNameBool = 'test-attr-bool';
+
+    test('should return the initial attribute value', () => {
+      const val = $host.getAttribute(attrName);
+      expect($el.$$attr(attrName)).toBe(val);
+    });
+
+    test('should set the attribute value and return the previous value', () => {
+      expect($el.$$attr(attrName, 'test')).toBe(null);
+      expect($el.$$attr(attrName)).toBe('test');
+    });
+
+    test('should set and return an empty string for boolean true', () => {
+      expect($el.$$attr(attrNameBool, true)).toBe(null);
+      expect($el.$$attr(attrNameBool)).toBe('');
+    });
+
+    test('should set and return null for removed attribute', () => {
+      expect($el.$$attr(attrNameBool, false)).toBe('');
+      expect($el.$$attr(attrNameBool)).toBe(null);
+    });
+  });
+
+  describe('ESLMixinElement.prototype.$$cls persist and correct', () => {
+    test('should return false when class does not exist', () => expect($el.$$cls('a')).toBe(false));
+
+    test('should return true when class exists', () => {
+      $host.className = 'a b';
+      expect($el.$$cls('a')).toBe(true);
+      expect($el.$$cls('a b')).toBe(true);
+    });
+
+    test('should add the class when setting to true', () => {
+      $host.className = '';
+      $el.$$cls('a', true);
+      $el.$$cls('b', true);
+      expect($host.className).toBe('a b');
+    });
+
+    test('should remove the class when setting to false', () => {
+      $host.className = 'a b';
+      expect($el.$$cls('a b', false)).toBe(false);
+      expect($host.className).toBe('');
+    });
+  });
+
+  test('ESLMixinElement.prototype.$$fire persist and correct', () => {
+    const eventName = 'testevent';
+    $host.dispatchEvent = jest.fn();
+    $el.$$fire(eventName);
+
+    expect($host.dispatchEvent).lastCalledWith(expect.objectContaining({type: eventName, cancelable: true, bubbles: true}));
+  });
+});

src/modules/esl-mixin-element/test/mixin.test.ts

@@ -2,11 +2,11 @@ import {ESLMixinElement} from '../core';
 
 // TODO: exited with more cases
 describe('ESLMixinElement', () => {
-  describe('register', () => {
-    class TestMixin extends ESLMixinElement {
-      static override is = 'test-mixin';
-    }
+  class TestMixin extends ESLMixinElement {
+    static override is = 'test-mixin';
+  }
 
+  describe('ESLMixinElement.register behaves according specification', () => {
     test('Existing ESLMixinElement attribute handled on registration', () => {
       const $el = document.createElement('div');
       $el.toggleAttribute(TestMixin.is, true);
@@ -59,28 +59,6 @@ describe('ESLMixinElement', () => {
       expect(TestMixin.get($el)).toBeInstanceOf(ESLMixinElement);
     });
 
-    test('ESLMixinElement can be resolved by mixin name', async () => {
-      const $el = document.createElement('div');
-      $el.toggleAttribute(TestMixin.is, true);
-      document.body.appendChild($el);
-      TestMixin.register();
-      await Promise.resolve(); // Wait for next microtask
-      expect(ESLMixinElement.get($el, TestMixin.is)).toBe(TestMixin.get($el));
-    });
-
-    test('init child', async () => {
-      TestMixin.register();
-
-      const $root = document.createElement('div');
-      document.body.appendChild($root);
-      const $el = document.createElement('div');
-      $root.appendChild($el);
-      $el.toggleAttribute(TestMixin.is, true);
-
-      await Promise.resolve(); // Wait for next microtask
-      expect(TestMixin.get($el)).toBeInstanceOf(ESLMixinElement);
-    });
-
     test('registration does not prevent appearing mixin from handling', async () => {
       class ATestMixin extends ESLMixinElement {
         static override is = 'a-test';
@@ -102,78 +80,96 @@ describe('ESLMixinElement', () => {
     });
   });
 
-  describe('ESLMixinElement prototype', () => {
-    class CTestMixin extends ESLMixinElement {
-      static override is = 'c-test';
-    }
+  describe('ESLMixinElement.get api present and behaves according specification', () => {
+    test('ESLMixinElement can be resolved by mixin name', async () => {
+      const $el = document.createElement('div');
+      $el.toggleAttribute(TestMixin.is, true);
+      document.body.appendChild($el);
+      TestMixin.register();
+      await Promise.resolve(); // Wait for next microtask
+      expect(ESLMixinElement.get($el, TestMixin.is)).toBe(TestMixin.get($el));
+    });
 
-    let $el: CTestMixin;
+    // TODO: more cases for ESLMixinElement.get
+  });
 
-    const $host = document.createElement('div');
-    $host.toggleAttribute(CTestMixin.is, true);
+  describe('ESLMixinRegistry handles mixins deep in modified DOM hierarchy', () => {
+    const $root = document.createElement('div');
+    const $el = document.createElement('div');
+    $el.toggleAttribute(TestMixin.is, true);
+    $root.appendChild($el);
 
     beforeAll(async () => {
-      document.body.appendChild($host);
-      CTestMixin.register();
+      TestMixin.register();
+      document.body.appendChild($root);
 
-      await Promise.resolve();
-      $el = CTestMixin.get($host) as CTestMixin;
+      await Promise.resolve(); // Wait for next microtask
     });
 
-    describe('ESLMixinElement $attr', () => {
-      const attrName = 'test-attr';
-      const attrNameBool = 'test-attr-bool';
+    test('ESLMixinRegistry initialize mixins on child elements of added DOM part', () => {
+      expect(TestMixin.get($el)).toBeInstanceOf(ESLMixinElement);
+    });
+
+    test('ESLMixinRegistry disconnect all mixins on removed DOM part', async () => {
+      $root.remove();
+      await Promise.resolve(); // Wait for next microtask
+      expect(TestMixin.get($el)).toBe(null);
+    });
+  });
 
-      test('should return the initial attribute value', () => {
-        const val = $host.getAttribute(attrName);
-        expect($el.$$attr(attrName)).toBe(val);
-      });
+  describe('ESLMixinRegistry handles multiple mixins deep in modified DOM hierarchy', () => {
+    class TestMixin2 extends ESLMixinElement {
+      static instanceCounter = 0;
+      static override is = 'test-mixin-2';
 
-      test('should set the attribute value and return the previous value', () => {
-        expect($el.$$attr(attrName, 'test')).toBe(null);
-        expect($el.$$attr(attrName)).toBe('test');
-      });
+      protected override connectedCallback(): void {
+        super.connectedCallback();
+        TestMixin2.instanceCounter++;
+      }
 
-      test('should set and return an empty string for boolean true', () => {
-        expect($el.$$attr(attrNameBool, true)).toBe(null);
-        expect($el.$$attr(attrNameBool)).toBe('');
-      });
+      protected override disconnectedCallback(): void {
+        super.disconnectedCallback();
+        TestMixin2.instanceCounter--;
+      }
+    }
 
-      test('should set and return null for removed attribute', () => {
-        expect($el.$$attr(attrNameBool, false)).toBe('');
-        expect($el.$$attr(attrNameBool)).toBe(null);
-      });
-    });
+    const $root = document.createElement('div');
+    const $el1 = document.createElement('div');
+    const $el2 = document.createElement('div');
+    $el1.toggleAttribute(TestMixin.is, true);
+    $el1.toggleAttribute(TestMixin2.is, true);
+    $el2.toggleAttribute(TestMixin2.is, true);
+    $el2.appendChild($el1);
+    $root.appendChild($el2);
 
-    describe('ESLMixinElement $cls', () => {
-      test('should return false when class does not exist', () => expect($el.$$cls('a')).toBe(false));
+    beforeAll(async () => {
+      TestMixin.register();
+      TestMixin2.register();
+      document.body.appendChild($root);
 
-      test('should return true when class exists', () => {
-        $host.className = 'a b';
-        expect($el.$$cls('a')).toBe(true);
-        expect($el.$$cls('a b')).toBe(true);
-      });
+      await Promise.resolve(); // Wait for next microtask
+    });
 
-      test('should add the class when setting to true', () => {
-        $host.className = '';
-        $el.$$cls('a', true);
-        $el.$$cls('b', true);
-        expect($host.className).toBe('a b');
-      });
+    test('ESLMixinRegistry initialize all mixins on child elements of added DOM part', () => {
+      expect(TestMixin.get($el1)).toBeInstanceOf(ESLMixinElement);
+      expect(TestMixin2.get($el1)).toBeInstanceOf(ESLMixinElement);
+      expect(TestMixin2.get($el2)).toBeInstanceOf(ESLMixinElement);
+    });
 
-      test('should remove the class when setting to false', () => {
-        $host.className = 'a b';
-        expect($el.$$cls('a b', false)).toBe(false);
-        expect($host.className).toBe('');
-      });
+    test('Test mixin instance counter is correct after elements creation', () => {
+      expect(TestMixin2.instanceCounter).toBe(2);
     });
 
-    test('ESLMixinElement $$fire', () => {
-      const eventName = 'testevent';
-      $host.dispatchEvent = jest.fn();
-      $el.$$fire(eventName);
+    test('ESLMixinRegistry disconnect all mixins on removed DOM part', async () => {
+      $root.remove();
+      await Promise.resolve(); // Wait for next microtask
+      expect(TestMixin.get($el1)).toBe(null);
+      expect(TestMixin2.get($el1)).toBe(null);
+      expect(TestMixin2.get($el2)).toBe(null);
+    });
 
-      expect($host.dispatchEvent).lastCalledWith(expect.objectContaining({type: eventName, cancelable: true, bubbles: true}));
+    test('Test mixin instance counter is correct after elements removal', () => {
+      expect(TestMixin2.instanceCounter).toBe(0);
     });
   });
 });

src/modules/esl-mixin-element/ui/esl-mixin-registry.ts

@@ -69,7 +69,7 @@ export class ESLMixinRegistry {
   public invalidateRecursive(root: HTMLElement = document.body, name?: string): void {
     if (!root) return;
     name ? this.invalidate(root, name) : this.invalidateAll(root);
-    if (!root.children || !root.children.length) return;
+    if (!root.children?.length) return;
     Array.prototype.forEach.call(root.children, (child: Element) => this.invalidateRecursive(child as HTMLElement, name));
   }
 
@@ -170,9 +170,11 @@ export class ESLMixinRegistry {
     delete store[mixin];
   }
 
-  /** Destroys all mixins on the element */
+  /** Destroys all mixins on the element and its subtree */
   private static destroyAll(el: HTMLElement): void {
     const store = (el as any)[STORE] as Record<string, ESLMixinElement> | undefined;
     store && Object.keys(store).forEach((name) => ESLMixinRegistry.destroy(el, name));
+    if (!el.children?.length) return;
+    Array.prototype.forEach.call(el.children, (child: Element) => ESLMixinRegistry.destroyAll(child as HTMLElement));
   }
 }