From 2a078ccd55cc8e1742b39aa2bc6b2c186351dd35 Mon Sep 17 00:00:00 2001 From: dmlvr Date: Wed, 2 Sep 2026 13:18:38 +0300 Subject: [PATCH 1/4] testing --- .../__internal/events/__tests__/click.test.ts | 69 +++++++++++++++++++ .../tests/DevExpress.ui.events/click.tests.js | 19 +++++ .../events.utils.nodesDisposing.tests.js | 29 +++++++- 3 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 packages/devextreme/js/__internal/events/__tests__/click.test.ts diff --git a/packages/devextreme/js/__internal/events/__tests__/click.test.ts b/packages/devextreme/js/__internal/events/__tests__/click.test.ts new file mode 100644 index 000000000000..609da2a82717 --- /dev/null +++ b/packages/devextreme/js/__internal/events/__tests__/click.test.ts @@ -0,0 +1,69 @@ +import { + afterEach, describe, expect, it, jest, +} from '@jest/globals'; +import { removeEvent } from '@js/common/core/events/remove'; +import eventsEngine from '@ts/events/core/m_events_engine'; + +import { name as clickEventName } from '../click'; + +type ElementEventData = Record; + +const noop = (): void => {}; + +const getRemoveHandlersCount = (element: Element): number => { + const elementData = eventsEngine.elementDataMap.get(element) as ElementEventData | undefined; + + return elementData?.[removeEvent]?.handleObjects.length ?? 0; +}; + +describe('dxclick nodes disposing (5025)', () => { + const clickableNodes: HTMLElement[] = []; + + const createClickableNode = (): HTMLElement => { + const node = document.createElement('div'); + + document.body.appendChild(node); + eventsEngine.on(node, clickEventName, noop); + clickableNodes.push(node); + + return node; + }; + + afterEach(() => { + clickableNodes.forEach((node) => { + eventsEngine.off(node); + node.remove(); + }); + clickableNodes.length = 0; + }); + + it('keeps a foreign dxremove handler on the previously clicked node', () => { + const clicked = createClickableNode(); + const other = createClickableNode(); + + clicked.click(); + + const foreignHandler = jest.fn(); + eventsEngine.on(clicked, removeEvent, foreignHandler); + + other.click(); + eventsEngine.triggerHandler(clicked, { type: removeEvent }); + + expect(foreignHandler).toHaveBeenCalledTimes(1); + }); + + it('removes only its own dxremove handler from the previously clicked node', () => { + const clicked = createClickableNode(); + const other = createClickableNode(); + + clicked.click(); + expect(getRemoveHandlersCount(clicked)).toBe(1); + + eventsEngine.on(clicked, removeEvent, noop); + expect(getRemoveHandlersCount(clicked)).toBe(2); + + other.click(); + + expect(getRemoveHandlersCount(clicked)).toBe(1); + }); +}); diff --git a/packages/devextreme/testing/tests/DevExpress.ui.events/click.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.events/click.tests.js index b1a60379837d..7d16464a8136 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.events/click.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.events/click.tests.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import { noop } from 'core/utils/common'; import clickEvent from 'common/core/events/click'; +import { removeEvent } from 'common/core/events/remove'; import domUtils from '__internal/core/utils/m_dom'; import support from '__internal/core/utils/m_support'; import devices from '__internal/core/m_devices'; @@ -425,3 +426,21 @@ QUnit.test('dxclick should not be fired twice when \'click\' is triggered from i pointer.start().down().up(); $(document).off('dxclick', $.noop); }); + +QUnit.test('foreign dxremove handler on the previously clicked node should survive a dxclick on another node (5025)', function(assert) { + const $clicked = $('#first').on('dxclick', noop); + const $other = $('#second').on('dxclick', noop); + let foreignHandlerCallCount = 0; + + nativePointerMock($clicked).start().click(); + + $clicked.on(removeEvent, function() { + foreignHandlerCallCount++; + }); + + nativePointerMock($other).start().click(); + + $clicked.triggerHandler({ type: removeEvent }); + + assert.equal(foreignHandlerCallCount, 1, `foreign ${removeEvent} handler is still subscribed`); +}); diff --git a/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js index 84e0709004f9..4bb91e3ab75b 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js @@ -31,7 +31,7 @@ QUnit.test('should clean elementDataMap when using subscribeNodesDisposing and u ? afterSubscribeElementData[removeEvent].handleObjects.length : 0; - unsubscribeNodesDisposing(clickEvent, subscriptionData.callback, subscriptionData.nodes); + unsubscribeNodesDisposing(clickEvent, subscriptionData.onceCallback, subscriptionData.nodes); const finalElementData = eventsEngine.elementDataMap.get(document); const afterUnsubscribeHandleObjectsCount = finalElementData && finalElementData[removeEvent] @@ -49,3 +49,30 @@ QUnit.test('should clean elementDataMap when using subscribeNodesDisposing and u `HandleObjects should be removed for "${removeEvent}" event after unsubscribe. HandleObjects count: ${afterUnsubscribeHandleObjectsCount};` ); }); + +QUnit.test('unsubscribeNodesDisposing should remove only the passed handler (5025)', function(assert) { + const testElement = document.getElementById('test-element'); + let subscribedCallbackCallCount = 0; + let foreignHandlerCallCount = 0; + + const clickEvent = eventsEngine.Event('click', { + target: testElement, + currentTarget: testElement, + delegateTarget: testElement + }); + + const subscriptionData = subscribeNodesDisposing(clickEvent, function() { + subscribedCallbackCallCount++; + }); + + eventsEngine.on(testElement, removeEvent, function() { + foreignHandlerCallCount++; + }); + + unsubscribeNodesDisposing(clickEvent, subscriptionData.onceCallback, subscriptionData.nodes); + + eventsEngine.triggerHandler(testElement, { type: removeEvent }); + + assert.equal(foreignHandlerCallCount, 1, `foreign "${removeEvent}" handler should be kept`); + assert.equal(subscribedCallbackCallCount, 0, `subscribed "${removeEvent}" handler should be removed`); +}); From 37e9e65ad26294a0c44494eb7704998c968f10c9 Mon Sep 17 00:00:00 2001 From: dmlvr Date: Wed, 2 Sep 2026 13:36:28 +0300 Subject: [PATCH 2/4] fix --- packages/devextreme/js/__internal/events/click.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/devextreme/js/__internal/events/click.ts b/packages/devextreme/js/__internal/events/click.ts index 51a61fb9a9ca..7b715df3bd35 100644 --- a/packages/devextreme/js/__internal/events/click.ts +++ b/packages/devextreme/js/__internal/events/click.ts @@ -46,12 +46,12 @@ const clickHandler = function (e: EmitterEvent & { originalEvent: NativeClickEve } if (lastFiredEvent && subscriptions.has(lastFiredEvent)) { - // @ts-expect-error the subscription stores onceCallback, not callback, so this - // destructured callback is always undefined and off() drops every dxremove - // handler from the nodes - const { nodes, callback } = subscriptions.get(lastFiredEvent) as NodesDisposingSubscription; + const { + nodes, + onceCallback, + } = subscriptions.get(lastFiredEvent) as NodesDisposingSubscription; - unsubscribeNodesDisposing(lastFiredEvent, callback, nodes); + unsubscribeNodesDisposing(lastFiredEvent, onceCallback, nodes); subscriptions.delete(lastFiredEvent); } From c4adf19628528726b31601f13a7208fbe9ec9465 Mon Sep 17 00:00:00 2001 From: dmlvr Date: Wed, 2 Sep 2026 14:41:21 +0300 Subject: [PATCH 3/4] refactoring --- .../events.utils.nodesDisposing.tests.js | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js index 4bb91e3ab75b..888a11bf16ca 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js @@ -49,30 +49,3 @@ QUnit.test('should clean elementDataMap when using subscribeNodesDisposing and u `HandleObjects should be removed for "${removeEvent}" event after unsubscribe. HandleObjects count: ${afterUnsubscribeHandleObjectsCount};` ); }); - -QUnit.test('unsubscribeNodesDisposing should remove only the passed handler (5025)', function(assert) { - const testElement = document.getElementById('test-element'); - let subscribedCallbackCallCount = 0; - let foreignHandlerCallCount = 0; - - const clickEvent = eventsEngine.Event('click', { - target: testElement, - currentTarget: testElement, - delegateTarget: testElement - }); - - const subscriptionData = subscribeNodesDisposing(clickEvent, function() { - subscribedCallbackCallCount++; - }); - - eventsEngine.on(testElement, removeEvent, function() { - foreignHandlerCallCount++; - }); - - unsubscribeNodesDisposing(clickEvent, subscriptionData.onceCallback, subscriptionData.nodes); - - eventsEngine.triggerHandler(testElement, { type: removeEvent }); - - assert.equal(foreignHandlerCallCount, 1, `foreign "${removeEvent}" handler should be kept`); - assert.equal(subscribedCallbackCallCount, 0, `subscribed "${removeEvent}" handler should be removed`); -}); From 5abfc771acaf6e1f408d63d731bfd991ac57acde Mon Sep 17 00:00:00 2001 From: dmlvr Date: Wed, 2 Sep 2026 15:08:18 +0300 Subject: [PATCH 4/4] fix memory leak --- .../devextreme/js/__internal/events/click.ts | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/devextreme/js/__internal/events/click.ts b/packages/devextreme/js/__internal/events/click.ts index 7b715df3bd35..c8d16f7387e4 100644 --- a/packages/devextreme/js/__internal/events/click.ts +++ b/packages/devextreme/js/__internal/events/click.ts @@ -28,10 +28,11 @@ interface NodesDisposingSubscription { let prevented: boolean | null = null; let lastFiredEvent: NativeClickEvent | null = null; -const subscriptions = new Map(); +let lastSubscription: NodesDisposingSubscription | null = null; const onNodeRemove = (): void => { lastFiredEvent = null; + lastSubscription = null; }; const clickHandler = function (e: EmitterEvent & { originalEvent: NativeClickEvent }): void { @@ -45,25 +46,19 @@ const clickHandler = function (e: EmitterEvent & { originalEvent: NativeClickEve originalEvent.DXCLICK_FIRED = true; } - if (lastFiredEvent && subscriptions.has(lastFiredEvent)) { - const { - nodes, - onceCallback, - } = subscriptions.get(lastFiredEvent) as NodesDisposingSubscription; + if (lastFiredEvent && lastSubscription) { + const { nodes, onceCallback } = lastSubscription; unsubscribeNodesDisposing(lastFiredEvent, onceCallback, nodes); - subscriptions.delete(lastFiredEvent); + lastSubscription = null; } lastFiredEvent = originalEvent; - const subscriptionData: NodesDisposingSubscription = subscribeNodesDisposing( - lastFiredEvent, - onNodeRemove, - ); - - subscriptions.set(lastFiredEvent, subscriptionData); + if (originalEvent) { + lastSubscription = subscribeNodesDisposing(originalEvent, onNodeRemove); + } fireEvent({ type: CLICK_EVENT_NAME,