From b304c52a30d86d462e2d5e03b00353b7bbf39cb7 Mon Sep 17 00:00:00 2001 From: "ala'n (Alexey Stsefanovich)" Date: Wed, 8 May 2024 22:35:00 +0200 Subject: [PATCH] fix(esl-event-listener): fix re-subscription when condition is used Note: warnings for incorrect subscription is missing --- src/modules/esl-event-listener/core/api.ts | 12 +- .../esl-event-listener/core/listener.ts | 5 +- .../test/listener.resubscribe.test.ts | 131 +++++++++++++++--- .../test/targets/decorator.test.ts | 6 +- 4 files changed, 115 insertions(+), 39 deletions(-) diff --git a/src/modules/esl-event-listener/core/api.ts b/src/modules/esl-event-listener/core/api.ts index 963afa600..426779d2b 100644 --- a/src/modules/esl-event-listener/core/api.ts +++ b/src/modules/esl-event-listener/core/api.ts @@ -1,5 +1,4 @@ import {ExportNs} from '../../esl-utils/environment/export-ns'; -import {resolveProperty} from '../../esl-utils/misc/functions'; import {dispatchCustomEvent} from '../../esl-utils/dom/events/misc'; import {ESLEventListener} from './listener'; @@ -95,10 +94,7 @@ export class ESLEventUtils { ); } const desc = typeof eventDesc === 'string' ? {event: eventDesc} : eventDesc as ESLListenerDescriptor; - if (Object.hasOwnProperty.call(desc, 'condition') && !resolveProperty(desc.condition, host)) return []; - const listeners = ESLEventListener.subscribe(host, handler, desc); - if (!listeners.length) emptySubscriptionWarning(host, desc, handler); - return listeners; + return ESLEventListener.subscribe(host, handler, desc); } /** @@ -109,11 +105,5 @@ export class ESLEventUtils { public static unsubscribe = ESLEventListener.unsubscribe; } -function emptySubscriptionWarning(host: object, descriptor: ESLListenerDescriptor, handler: ESLListenerHandler): void { - const event = resolveProperty(descriptor.event, host); - const target = resolveProperty(descriptor.target, host); - console.warn('[ESL]: Empty subscription %o', {host, descriptor, handler, event, target}); -} - /** @deprecated alias for {@link ESLEventUtils} */ export const EventUtils = ESLEventUtils; diff --git a/src/modules/esl-event-listener/core/listener.ts b/src/modules/esl-event-listener/core/listener.ts index 1193c5bf7..4e4da966a 100644 --- a/src/modules/esl-event-listener/core/listener.ts +++ b/src/modules/esl-event-listener/core/listener.ts @@ -42,6 +42,7 @@ export const splitEvents = (events: string): string[] => { * */ export class ESLEventListener implements ESLListenerDefinition, EventListenerObject { public readonly target?: ESLListenerTarget | PropertyProvider; + public readonly condition?: PropertyProvider; public readonly selector?: string; public readonly capture?: boolean; @@ -130,8 +131,8 @@ export class ESLEventListener implements ESLListenerDefinition, EventListenerObj public subscribe(): boolean { const {passive, capture} = this; this.unsubscribe(); - memoize.clear(this, '$targets'); - memoize.clear(this, 'handleEvent'); + memoize.clear(this, ['$targets', 'handleEvent']); + if (resolveProperty(this.condition, this.host) === false) return false; if (!this.$targets.length) return false; this.$targets.forEach((el: EventTarget) => el.addEventListener(this.event, this, {passive, capture})); ESLEventListener.add(this.host, this); diff --git a/src/modules/esl-event-listener/test/listener.resubscribe.test.ts b/src/modules/esl-event-listener/test/listener.resubscribe.test.ts index aecb1282d..5704a3c51 100644 --- a/src/modules/esl-event-listener/test/listener.resubscribe.test.ts +++ b/src/modules/esl-event-listener/test/listener.resubscribe.test.ts @@ -1,34 +1,119 @@ import {ESLEventUtils} from '../core/api'; + describe('ESLEventUtils.subscribe resubscribing event', () => { const $host = document.createElement('div'); - const handle = jest.fn(); - const targ1 = document.createElement('button'); - const targ2 = document.createElement('button'); - const target = jest.fn(() => targ1); - - const listeners1 = ESLEventUtils.subscribe($host, {event: 'click', target}, handle); - test('ESLEventUtils.subscribe subscription correct first time', () => { - expect(listeners1.length).toBe(1); - expect(ESLEventUtils.listeners($host).length).toBe(1); - }); + const $el1 = document.createElement('button'); + const $el2 = document.createElement('button'); - target.mockImplementation(() => targ2); - const listeners2 = ESLEventUtils.subscribe($host, {event: 'click', target}, handle); - test('ESLEventUtils.subscribe subscription correct second time', () => { - expect(listeners2.length).toBe(1); - expect(ESLEventUtils.listeners($host).length).toBe(1); - }); + describe('ESLEventUtils.subscribe dynamic resubscribtion of the same event with different target', () => { + const handle = jest.fn(); + const target = jest.fn(() => $el1); + + const listeners1 = ESLEventUtils.subscribe($host, {event: 'click', target}, handle); + test('ESLEventUtils.subscribe subscription correct first time', () => { + expect(listeners1.length).toBe(1); + expect(ESLEventUtils.listeners($host)).toEqual(listeners1); + }); + + // Change target + target.mockImplementation(() => $el2); + + // Subscription descriptor is the same as produced in listener 1 situation + const listeners2 = ESLEventUtils.subscribe($host, {event: 'click', target}, handle); + test('ESLEventUtils.subscribe subscription correct second time', () => { + expect(listeners2.length).toBe(1); + expect(ESLEventUtils.listeners($host)).toEqual(listeners2); + }); - test('Resubscribed listener does not observe initial target', () => { - targ1.click(); - expect(handle).not.toBeCalled(); + test('Resubscribed listener does not observe initial target', () => { + $el1.click(); + expect(handle).not.toHaveBeenCalled(); + }); + + test('Resubscribed listener observes actual target', () => { + $el2.click(); + expect(handle).toHaveBeenCalled(); + }); + + test('ESLEventUtils does not recreate instance', () => expect(listeners1).toEqual(listeners2)); }); - test('Resubscribed listener observes actual target', () => { - targ2.click(); - expect(handle).toBeCalled(); + describe('ESLEventUtils.subscribe resubscribes listener correctly if the source is declared descriptor', () => { + class Test { + onClick() {} + } + + const target = jest.fn(() => $el1); + ESLEventUtils.initDescriptor(Test.prototype, 'onClick', {event: 'click', auto: true, target}); + + const instance = new Test(); + ESLEventUtils.subscribe(instance); + + test('Initial subscription happened', () => { + expect(ESLEventUtils.listeners(instance).length).toBe(1); + }); + + test('Initial subscription have a correct target', () => { + expect(ESLEventUtils.listeners(instance)[0].$targets).toEqual([$el1]); + }); + + // Change target + target.mockImplementation(() => $el2); + // Re-subscription + ESLEventUtils.subscribe(instance); + + test('Re-subscription does not create new listener', () => { + expect(ESLEventUtils.listeners(instance).length).toBe(1); + }); + + test('Re-subscription have a correct target', () => { + expect(ESLEventUtils.listeners(instance)[0].$targets).toEqual([$el2]); + }); }); - test('ESLEventUtils does not recreate instance', () => expect(listeners1[0]).toBe(listeners2[0])); + describe('ESLEventUtils.subscribe resubscribes listener correctly if the condition is involved', () => { + class Test { + allowed = false; + onClick() {} + } + ESLEventUtils.initDescriptor(Test.prototype, 'onClick', { + auto: true, + event: 'click', + target: $el1, + condition: ($that: Test) => $that.allowed + }); + + describe('Dynamic condition change to false', () => { + const instance = new Test(); + + test('Initial subscription happened (condition: true)', () => { + instance.allowed = true; + ESLEventUtils.subscribe(instance); + expect(ESLEventUtils.listeners(instance).length).toBe(1); + }); + + test('Subscription removed when condition dynamically ', () => { + instance.allowed = false; + ESLEventUtils.subscribe(instance); + expect(ESLEventUtils.listeners(instance).length).toBe(0); + }); + }); + + describe('Dynamic condition change to true', () => { + const instance = new Test(); + + test('Initial subscription does not happened (condition: false)', () => { + instance.allowed = false; + ESLEventUtils.subscribe(instance); + expect(ESLEventUtils.listeners(instance).length).toBe(0); + }); + + test('Subscription added when condition dynamically ', () => { + instance.allowed = true; + ESLEventUtils.subscribe(instance); + expect(ESLEventUtils.listeners(instance).length).toBe(1); + }); + }); + }); }); diff --git a/src/modules/esl-event-listener/test/targets/decorator.test.ts b/src/modules/esl-event-listener/test/targets/decorator.test.ts index 9f40d77cd..f7653f42f 100644 --- a/src/modules/esl-event-listener/test/targets/decorator.test.ts +++ b/src/modules/esl-event-listener/test/targets/decorator.test.ts @@ -53,9 +53,9 @@ describe('ESLDecoratedEventTarget proxy', () => { test('Creation happens once on subscription', () => { const fn = jest.fn(); - decorated.addEventListener(fn); - expect(dec).toBeCalled(); - expect(handler).not.toBeCalled(); + decorated.addEventListener('something', fn); + expect(dec).toHaveBeenCalled(); + expect(handler).not.toHaveBeenCalled(); decorated.removeEventListener(fn); });