From d59c7d81c0cab02d1f3a90bbfff2221227816e3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Franc=CC=A7ois=20Meinesz?= Date: Fri, 26 Apr 2019 17:28:47 +0100 Subject: [PATCH 1/7] Attempt to solve Memory leaks - Detached HTML elements Attempt to solve Memory leaks - Detached HTML elements --- packages/core/InteractableSet.ts | 4 ++++ packages/core/Interaction.ts | 6 ++++++ packages/core/scope.ts | 3 +++ 3 files changed, 13 insertions(+) diff --git a/packages/core/InteractableSet.ts b/packages/core/InteractableSet.ts index 1f3e8f0f9..aab4fb7f4 100644 --- a/packages/core/InteractableSet.ts +++ b/packages/core/InteractableSet.ts @@ -22,6 +22,10 @@ export default class InteractableSet { : target[this.scope.id] targetMappings.splice(targetMappings.findIndex((m) => m.context === context), 1) + if (interactable.target[scope.id]) { + interactable.target[scope.id].context = null + interactable.target[scope.id].interactable = null + } }) } diff --git a/packages/core/Interaction.ts b/packages/core/Interaction.ts index df0adda5a..7ac473f13 100644 --- a/packages/core/Interaction.ts +++ b/packages/core/Interaction.ts @@ -468,6 +468,12 @@ export class Interaction { this._latestPointer.eventTarget = eventTarget } + destroy () { + this._latestPointer.pointer = null + this._latestPointer.event = null + this._latestPointer.eventTarget = null + } + _createPreparedEvent (event: Interact.PointerEventType, phase: EventPhase, preEnd: boolean, type: string) { const actionName = this.prepared.name diff --git a/packages/core/scope.ts b/packages/core/scope.ts index bb052aca2..366fe06c8 100644 --- a/packages/core/scope.ts +++ b/packages/core/scope.ts @@ -92,8 +92,11 @@ export class Scope { if (interaction.interactable === this) { interaction.stop() } + interaction.destroy() } + scope.interactions.list = [] + scope.interactables.signals.fire('unset', { interactable: this }) } } From 93e0eecc172a85257ea3a4f7a604353af8e603c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Franc=CC=A7ois=20Meinesz?= Date: Fri, 26 Apr 2019 17:58:45 +0100 Subject: [PATCH 2/7] Test script MacOS Compatibility fix --- scripts/test.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/test.sh b/scripts/test.sh index e8b0d3435..3ffa03f90 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,5 +1,9 @@ #!/bin/bash PKG_DIR=$(dirname $(dirname $(readlink -f $0))) +if [ -z "$PKG_DIR" ] +then + PKG_DIR=$(dirname $(dirname $0)) +fi export PATH=$PKG_DIR/node_modules/.bin:$PWD/node_modules/.bin:$PATH export NODE_ENV=test From 5144f9723679cac2c0a9dbdc564bdab47f1fec2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Franc=CC=A7ois=20Meinesz?= Date: Fri, 26 Apr 2019 22:19:52 +0100 Subject: [PATCH 3/7] Auto-Scroll Memory-leak fix --- packages/auto-scroll/index.ts | 8 ++++++++ packages/core/scope.ts | 1 + 2 files changed, 9 insertions(+) diff --git a/packages/auto-scroll/index.ts b/packages/auto-scroll/index.ts index ae3cd790e..701a58e7b 100644 --- a/packages/auto-scroll/index.ts +++ b/packages/auto-scroll/index.ts @@ -38,6 +38,14 @@ function install (scope: Scope) { interaction.autoScroll = null }) + interactions.signals.on('unset', ({ interaction }) => { + interaction.autoScroll = null + autoScroll.stop() + if (autoScroll.interaction) { + autoScroll.interaction = null + } + }) + interactions.signals.on('stop', autoScroll.stop) interactions.signals.on('action-move', (arg: any) => autoScroll.onInteractionMove(arg)) diff --git a/packages/core/scope.ts b/packages/core/scope.ts index 366fe06c8..30892db42 100644 --- a/packages/core/scope.ts +++ b/packages/core/scope.ts @@ -92,6 +92,7 @@ export class Scope { if (interaction.interactable === this) { interaction.stop() } + scope.interactions.signals.fire('unset', { interaction }) interaction.destroy() } From ec9f5227e8b3c316eeeac27fce192451e2f66470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Franc=CC=A7ois=20Meinesz?= Date: Mon, 29 Apr 2019 14:44:30 +0100 Subject: [PATCH 4/7] Prevent exception on unset when dropState is null --- packages/actions/drop/index.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/actions/drop/index.ts b/packages/actions/drop/index.ts index 6bf1dcad1..7a782138a 100644 --- a/packages/actions/drop/index.ts +++ b/packages/actions/drop/index.ts @@ -133,13 +133,15 @@ function install (scope: Scope) { const { dropState } = interaction - dropState.activeDrops = null - dropState.events = null - dropState.cur.dropzone = null - dropState.cur.element = null - dropState.prev.dropzone = null - dropState.prev.element = null - dropState.rejected = false + if (dropState) { + dropState.activeDrops = null + dropState.events = null + dropState.cur.dropzone = null + dropState.cur.element = null + dropState.prev.dropzone = null + dropState.prev.element = null + dropState.rejected = false + } }) /** From 2f5ad72faadd571bdddd3caf4b3f8f7739820b9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Franc=CC=A7ois=20Meinesz?= Date: Fri, 10 May 2019 11:14:08 +0100 Subject: [PATCH 5/7] Change 'unset' signal name to 'destroy' + correctly removing context and interactable reference from interactable target --- packages/auto-scroll/index.ts | 2 +- packages/core/InteractableSet.ts | 10 ++++++---- packages/core/scope.ts | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/auto-scroll/index.ts b/packages/auto-scroll/index.ts index 701a58e7b..3b6cb7d2b 100644 --- a/packages/auto-scroll/index.ts +++ b/packages/auto-scroll/index.ts @@ -38,7 +38,7 @@ function install (scope: Scope) { interaction.autoScroll = null }) - interactions.signals.on('unset', ({ interaction }) => { + interactions.signals.on('destroy', ({ interaction }) => { interaction.autoScroll = null autoScroll.stop() if (autoScroll.interaction) { diff --git a/packages/core/InteractableSet.ts b/packages/core/InteractableSet.ts index aab4fb7f4..a610271b4 100644 --- a/packages/core/InteractableSet.ts +++ b/packages/core/InteractableSet.ts @@ -21,11 +21,13 @@ export default class InteractableSet { ? this.selectorMap[target] : target[this.scope.id] - targetMappings.splice(targetMappings.findIndex((m) => m.context === context), 1) - if (interactable.target[scope.id]) { - interactable.target[scope.id].context = null - interactable.target[scope.id].interactable = null + const targetIndex = targetMappings.findIndex((m) => m.context === context) + if (targetMappings[targetIndex]) { + // Destroying mappingInfo's context and interactable + targetMappings[targetIndex].context = null + targetMappings[targetIndex].interactable = null } + targetMappings.splice(targetIndex, 1) }) } diff --git a/packages/core/scope.ts b/packages/core/scope.ts index 30892db42..58bb4cdf5 100644 --- a/packages/core/scope.ts +++ b/packages/core/scope.ts @@ -92,7 +92,7 @@ export class Scope { if (interaction.interactable === this) { interaction.stop() } - scope.interactions.signals.fire('unset', { interaction }) + scope.interactions.signals.fire('destroy', { interaction }) interaction.destroy() } From 7b9e19124a6210235f0343f27d575aa01f82167f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Franc=CC=A7ois=20Meinesz?= Date: Fri, 10 May 2019 11:17:27 +0100 Subject: [PATCH 6/7] Add unit tests for unset and destroy --- packages/core/Interactable.spec.ts | 23 +++++++++++++++++++++++ packages/core/Interaction.spec.ts | 23 ++++++++++++++++++++++- packages/interact/interact.spec.ts | 2 ++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/core/Interactable.spec.ts b/packages/core/Interactable.spec.ts index 99d845539..87f3732b6 100644 --- a/packages/core/Interactable.spec.ts +++ b/packages/core/Interactable.spec.ts @@ -36,6 +36,29 @@ test('Interactable copies and extends defaults', (t) => { t.end() }) +test('Interactable unset correctly', (t) => { + const scope = helpers.mockScope() as any + const { defaults } = scope + + const div = d('div') + const interactable = scope.interactables.new(div) + + const mappingInfo = div[scope.id][0] + + scope.interactables.signals.fire('unset', { interactable }) + + t.strictEqual(mappingInfo.context, null, + 'unset mappingInfo context') + + t.strictEqual(mappingInfo.interactable, null, + 'unset mappingInfo interactable') + + t.strictEqual(div[scope.id].length, 0, + 'unset target are removed') + + t.end() +}) + test('Interactable copies and extends per action defaults', (t) => { const scope = helpers.mockScope() const { defaults } = scope diff --git a/packages/core/Interaction.spec.ts b/packages/core/Interaction.spec.ts index d3aa87a2b..2551ebb76 100644 --- a/packages/core/Interaction.spec.ts +++ b/packages/core/Interaction.spec.ts @@ -30,7 +30,7 @@ test('Interaction constructor', (t) => { for (const coordField in interaction.coords) { t.deepEqual(interaction.coords[coordField], zeroCoords, - `nteraction.coords.${coordField} set to zero`) + `interaction.coords.${coordField} set to zero`) } t.equal(interaction.pointerType, testType, @@ -50,6 +50,27 @@ test('Interaction constructor', (t) => { t.end() }) +test('Interaction destroy', (t) => { + const interaction = makeInteractionAndSignals() + const pointer = { pointerId: 10 } as any + const event = {} as any + + interaction.updatePointer(pointer, event, null) + + interaction.destroy() + + t.strictEqual(interaction._latestPointer.pointer, null, + 'interaction._latestPointer.pointer is null') + + t.strictEqual(interaction._latestPointer.event, null, + 'interaction._latestPointer.event is null') + + t.strictEqual(interaction._latestPointer.eventTarget, null, + 'interaction._latestPointer.eventTarget is null') + + t.end() +}) + test('Interaction.getPointerIndex', (t) => { const interaction = makeInteractionAndSignals() diff --git a/packages/interact/interact.spec.ts b/packages/interact/interact.spec.ts index 318ba125c..64cc2d437 100644 --- a/packages/interact/interact.spec.ts +++ b/packages/interact/interact.spec.ts @@ -16,6 +16,8 @@ test('interact export', (t) => { interactable1.unset() t.equal(scope.interactables.list.length, 0, 'unset interactables are removed') + t.strictEqual(scope.interactions.list.length, 0, + 'unset interactions are removed') const constructsUniqueMessage = 'unique contexts make unique interactables with identical targets' From 21ef60cd68fa9edaacac1597056ab2888b263807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Franc=CC=A7ois=20Meinesz?= Date: Fri, 10 May 2019 11:19:10 +0100 Subject: [PATCH 7/7] Add unit tests for unset and destroy --- packages/core/Interactable.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/Interactable.spec.ts b/packages/core/Interactable.spec.ts index 87f3732b6..0c4cff35f 100644 --- a/packages/core/Interactable.spec.ts +++ b/packages/core/Interactable.spec.ts @@ -38,7 +38,6 @@ test('Interactable copies and extends defaults', (t) => { test('Interactable unset correctly', (t) => { const scope = helpers.mockScope() as any - const { defaults } = scope const div = d('div') const interactable = scope.interactables.new(div)