Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions src/modules/esl-mixin-element/test/mixin.prototype.test.ts
Original file line number Diff line number Diff line change
@@ -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}));
});
});
156 changes: 76 additions & 80 deletions src/modules/esl-mixin-element/test/mixin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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';
Expand All @@ -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);
});
});
});
4 changes: 3 additions & 1 deletion src/modules/esl-mixin-element/ui/esl-mixin-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 || !el.children.length) return;
ala-n marked this conversation as resolved.
Show resolved Hide resolved
Array.prototype.forEach.call(el.children, (child: Element) => ESLMixinRegistry.destroyAll(child as HTMLElement));
}
}
Loading