Repository: exadel-inc/esl
Pull request #1144 (2022-08-10 15:18:43)

feat(esl-utility): event listener subscription logic fix and ability to inherit listener definition
In bounds of this PR:
1. Add a check to prevent an empty targets listener from subscription
2. Make able to inherit event listener method definition meta from superclass with a short version of `@listen` decorator
3. (cosmetic) `prop` decorator added to the `esl-mixin-element` and `esl-base-element` modules export
4. cover new functionality by tests

The following inheritance cases are available :
For now the following syntax is available:
```
class Super {
   @listen({...})
   onEvent() {}
}

// Unsubscribe option 
class ClassA {
   // overrides without listener definition
   onEvent() {}
}

// Replaces Super definition
class ClassB {
   @listen({...})
   onEvent() {}
}

// Inherit and merge Super event listener definition
class ClassC {
   @listen({inherit: true, ...})
   onEvent() {}
}

// Just inherit Super event listener definition
class ClassD {
   @listen({inherit: true})
   onEvent() {}
}

// Sort version to just inherit Super event listener definition
class ClassE {
   @listen()
   onEvent() {}
}
```

----

@exadel-inc/esl-all-contributors, please feel free to leave feedback about the current implementation.

Also, please take a look at the inheritance use cases described above, and please let me know:

1. Is this behavior looks safe and intuitive for you in general?
(Short emoji: 👍🏻(yes) / 👎🏻 (no) )
2. Do we need to have a short version to inherit listener metadata,
or we should restrict case of ClassE to the full version (ClassD)
(Short emoji: 🚀 (short version ok) / 😕 (short version confused) )


Patch:
diff --git a/src/modules/esl-base-element/core.ts b/src/modules/esl-base-element/core.ts
index 6ca2519b9..dabbc5d7c 100644
--- a/src/modules/esl-base-element/core.ts
+++ b/src/modules/esl-base-element/core.ts
@@ -2,6 +2,7 @@ export type {ESLBaseElementShape} from './core/esl-base-element.shape';
 
 export * from './core/esl-base-element';
 
+export * from '../esl-utils/decorators/prop';
 export * from '../esl-utils/decorators/attr';
 export * from '../esl-utils/decorators/bool-attr';
 export * from '../esl-utils/decorators/json-attr';
diff --git a/src/modules/esl-mixin-element/core.ts b/src/modules/esl-mixin-element/core.ts
index 6bcc6564e..de26e22f8 100644
--- a/src/modules/esl-mixin-element/core.ts
+++ b/src/modules/esl-mixin-element/core.ts
@@ -1,5 +1,6 @@
 export * from './ui/esl-mixin-element';
 
+export * from '../esl-utils/decorators/prop';
 export * from '../esl-utils/decorators/attr';
 export * from '../esl-utils/decorators/bool-attr';
 export * from '../esl-utils/decorators/json-attr';
diff --git a/src/modules/esl-utils/decorators/listen.ts b/src/modules/esl-utils/decorators/listen.ts
index 8056fd051..28193fbf9 100644
--- a/src/modules/esl-utils/decorators/listen.ts
+++ b/src/modules/esl-utils/decorators/listen.ts
@@ -6,6 +6,11 @@ import type {ESLListenerHandler, ESLListenerEventMap, ESLListenerDescriptor} fro
 type ListenDecorator<EType extends Event> =
   (target: any, property: string, descriptor: TypedPropertyDescriptor<ESLListenerHandler<EType>>) => void;
 
+type ESLListenerDescriptorExt<T extends keyof ESLListenerEventMap = string> = Partial<ESLListenerDescriptor<T>> & {
+  /** Defines if the listener metadata should be inherited from the method of the superclass */
+  inherit?: boolean;
+};
+
 /**
  * Decorator to declare listener ({@link ESLEventListener}) meta information
  * Defines auto-subscribable event
@@ -17,14 +22,15 @@ export function listen<K extends keyof ESLListenerEventMap>(event: K | PropertyP
  * Defines auto-subscribable event by default
  * @param desc - event listener configuration {@link ESLListenerDescriptor}
  */
-export function listen<K extends keyof ESLListenerEventMap>(desc: ESLListenerDescriptor<K>): ListenDecorator<ESLListenerEventMap[K]>;
+export function listen<K extends keyof ESLListenerEventMap>(desc: ESLListenerDescriptorExt<K>): ListenDecorator<ESLListenerEventMap[K]>;
 
-export function listen(desc: string | ESLListenerDescriptor): ListenDecorator<Event> {
+export function listen(desc: string | ESLListenerDescriptorExt): ListenDecorator<Event> {
   return function listener<T extends ESLListenerHandler>(target: HTMLElement,
                                                          propertyKey: string,
                                                          descriptor: TypedPropertyDescriptor<T>): void {
+    const superDesc = Object.getPrototypeOf(target)[propertyKey];
     desc = typeof desc === 'string' || typeof desc === 'function' ? {event: desc} : desc;
-    desc = Object.assign({auto: true}, desc);
+    desc = Object.assign({auto: true}, desc.inherit && isDescriptorFn(superDesc) ? superDesc : {}, desc);
 
     if (!descriptor || typeof descriptor.value !== 'function') {
       throw new TypeError('Only class methods can be decorated via listener decorator');
diff --git a/src/modules/esl-utils/dom/events/listener.ts b/src/modules/esl-utils/dom/events/listener.ts
index 0b7b72180..f79e3bd0e 100644
--- a/src/modules/esl-utils/dom/events/listener.ts
+++ b/src/modules/esl-utils/dom/events/listener.ts
@@ -129,13 +129,18 @@ export class ESLEventListener implements ESLListenerDescriptor, EventListenerObj
     return current.contains(target.closest(this.selector));
   }
 
-  /** (Re-)Subscribes event listener instance */
-  public subscribe(): void {
+  /**
+   * (Re-)Subscribes event listener instance
+   * @returns if subscription was successful
+   */
+  public subscribe(): boolean {
     const {passive, capture} = this;
     this.unsubscribe();
     memoize.clear(this, '$targets');
+    if (!this.$targets.length) return false;
     this.$targets.forEach((el: EventTarget) => el.addEventListener(this.event, this, {passive, capture}));
     ESLEventListener.get(this.$host).push(this);
+    return true;
   }
 
   /** Unsubscribes event listener instance */
@@ -157,6 +162,9 @@ export class ESLEventListener implements ESLListenerDescriptor, EventListenerObj
   /** Creates event listeners by handler and descriptors */
   public static create(target: HTMLElement, handler: ESLListenerHandler, desc: ESLListenerDescriptor): ESLEventListener[] {
     const events = resolveProperty(desc.event, desc.context || target);
+    if (!events.length) {
+      console.warn('[ESL]: Can\'t create ESLEventListener with empty event type: %o %o', handler, desc);
+    }
     return ESLEventListener.splitEventQ(events).map((event) => {
       const spec: ESLListenerDescriptor = Object.assign({}, desc, {event});
       return new ESLEventListener(target, handler, spec);
diff --git a/src/modules/esl-utils/dom/scroll.less b/src/modules/esl-utils/dom/scroll.less
index c5025b59d..ee8b8b5d4 100644
--- a/src/modules/esl-utils/dom/scroll.less
+++ b/src/modules/esl-utils/dom/scroll.less
@@ -8,15 +8,13 @@
 
 html[esl-scroll-lock] body {
   overflow: hidden !important;
+
+  max-width: 100vw !important;
+  max-height: 100vh !important;
 }
 
 html[esl-scroll-lock='native'] {
   overflow-y: scroll !important;
-
-  body {
-    max-width: 100vw !important;
-    max-height: 100vh !important;
-  }
 }
 
 html[esl-scroll-lock='pseudo'] {


Test patch:
diff --git a/src/modules/esl-utils/decorators/test/listen.test.ts b/src/modules/esl-utils/decorators/test/listen.test.ts
index 5b095602e..d67a0adbd 100644
--- a/src/modules/esl-utils/decorators/test/listen.test.ts
+++ b/src/modules/esl-utils/decorators/test/listen.test.ts
@@ -2,9 +2,10 @@ import '../../../../polyfills/es5-target-shim';
 
 import {listen} from '../listen';
 import {EventUtils} from '../../dom/events';
+import type {ESLListenerDescriptorFn} from '../../dom/events';
 
-describe('Decorator: listen', () => {
-  test('short', () => {
+describe('Decorator: @listen', () => {
+  test('Decorator listen should accept one argument call with an event type', () => {
     class Test extends HTMLElement {
       @listen('click')
       onEvent() {}
@@ -16,7 +17,7 @@ describe('Decorator: listen', () => {
     expect(EventUtils.descriptors(test)[0].event).toBe('click');
   });
 
-  test('provider', () => {
+  test('Decorator listen should accept one argument call with an event type provider', () => {
     class Test extends HTMLElement {
       @listen(() => 'test')
       onEvent() {}
@@ -28,7 +29,7 @@ describe('Decorator: listen', () => {
     expect(typeof EventUtils.descriptors(test)[0].event).toBe('function');
   });
 
-  test('full', () => {
+  test('Multiple @listen declarations should be auto-subscribed correctly with a full event definition', () => {
     class Test extends HTMLElement {
       @listen({event: 'event1'})
       onEvent1() {}
@@ -41,7 +42,7 @@ describe('Decorator: listen', () => {
     expect(EventUtils.descriptors(test).length).toBe(2);
   });
 
-  test('inheritance', () => {
+  test('Event listener definitions from the superclass should be handled correctly', () => {
     class Test extends HTMLElement {
       @listen({event: 'event1'})
       onEvent1() {}
@@ -56,7 +57,71 @@ describe('Decorator: listen', () => {
     expect(EventUtils.descriptors(test).length).toBe(2);
   });
 
-  test('multiple decoration', () => {
+  test('Override event listener definition without decorator should exclude subscription', () => {
+    class Test extends HTMLElement {
+      @listen({event: 'event1'})
+      onEvent() {}
+    }
+    class TestInh extends Test {
+      onEvent() {}
+    }
+    customElements.define('test-listen-4', TestInh);
+
+    const test = new TestInh();
+    expect(EventUtils.descriptors(test).length).toBe(0);
+  });
+
+  test('Override event listener definition with a @listen({inherit: true}) inherits event description meta information', () => {
+    class Test extends HTMLElement {
+      @listen({event: 'event1'})
+      onEvent() {}
+    }
+    class TestInh extends Test {
+      @listen({inherit: true})
+      onEvent() {}
+    }
+    customElements.define('test-listen-5', TestInh);
+
+    const test = new TestInh();
+    expect(EventUtils.descriptors(test).length).toBe(1);
+    expect((test.onEvent as any as ESLListenerDescriptorFn).event).toBe('event1');
+  });
+
+  test('Override event listener definition with a @listen({inherit: true, ...}) should merge event description meta information', () => {
+    class Test extends HTMLElement {
+      @listen({event: 'event1', selector: 'e'})
+      onEvent() {}
+    }
+    class TestInh extends Test {
+      @listen({inherit: true, event: 'event2'})
+      onEvent() {}
+    }
+    customElements.define('test-listen-6', TestInh);
+
+    const test = new TestInh();
+    expect(EventUtils.descriptors(test).length).toBe(1);
+    expect((test.onEvent as any as ESLListenerDescriptorFn).event).toBe('event2');
+    expect((test.onEvent as any as ESLListenerDescriptorFn).selector).toBe('e');
+  });
+
+  test('Override event listener definition with a @listen({...}) replaces event description meta information', () => {
+    class Test extends HTMLElement {
+      @listen({event: 'event1', selector: 'e'})
+      onEvent() {}
+    }
+    class TestInh extends Test {
+      @listen({event: 'event2'})
+      onEvent() {}
+    }
+    customElements.define('test-listen-7', TestInh);
+
+    const test = new TestInh();
+    expect(EventUtils.descriptors(test).length).toBe(1);
+    expect((test.onEvent as any as ESLListenerDescriptorFn).event).toBe('event2');
+    expect((test.onEvent as any as ESLListenerDescriptorFn).selector).toBe(undefined);
+  });
+
+  test('Decorator @listen can not be applied multiple times to the same function', () => {
     expect(() => {
       class Test extends HTMLElement {
         @listen({event: 'event1'})
diff --git a/src/modules/esl-utils/dom/events/test/listener.test.ts b/src/modules/esl-utils/dom/events/test/listener.test.ts
index 273270712..fb3bff730 100644
--- a/src/modules/esl-utils/dom/events/test/listener.test.ts
+++ b/src/modules/esl-utils/dom/events/test/listener.test.ts
@@ -14,6 +14,18 @@ describe('dom/events: ESLEventListener', () => {
   });
 
   describe('create', () => {
+    test('without event type', () => {
+      const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => void 0);
+
+      const host = document.createElement('div');
+      const handler = jest.fn();
+      const result = ESLEventListener.create(host, handler, {event: ''});
+
+      expect(result.length).toBe(0);
+      expect(consoleSpy).toBeCalledTimes(1);
+      consoleSpy.mockClear();
+    });
+
     test('one by string', () => {
       const host = document.createElement('div');
       const handler = jest.fn();
diff --git a/src/modules/esl-utils/dom/events/test/utils.test.ts b/src/modules/esl-utils/dom/events/test/utils.test.ts
index 6d0e149af..307e352f8 100644
--- a/src/modules/esl-utils/dom/events/test/utils.test.ts
+++ b/src/modules/esl-utils/dom/events/test/utils.test.ts
@@ -89,6 +89,16 @@ describe('dom/events: EventUtils', () => {
       expect(createMock).toBeCalledWith(host, expect.anything(), expect.anything());
     });
 
+    test('decorated handler with empty override', () => {
+      const host = {};
+      const createMock =
+        jest.spyOn(ESLEventListener, 'create').mockImplementation((el, cb, desc) => [desc] as any);
+
+      EventUtils.subscribe(host as any, {}, listener1);
+      expect(listener1.subscribe).toBeCalled();
+      expect(createMock).toBeCalledWith(host, expect.anything(), expect.anything());
+    });
+
     test('merge decorated handler', () => {
       const host = {};
       const createMock =