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: Memory leak - Detached HTML elements #715

Merged
merged 7 commits into from
May 19, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 9 additions & 7 deletions packages/actions/drop/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
})

/**
Expand Down
8 changes: 8 additions & 0 deletions packages/auto-scroll/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ function install (scope: Scope) {
interaction.autoScroll = null
})

interactions.signals.on('destroy', ({ 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))
Expand Down
22 changes: 22 additions & 0 deletions packages/core/Interactable.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,28 @@ test('Interactable copies and extends defaults', (t) => {
t.end()
})

test('Interactable unset correctly', (t) => {
const scope = helpers.mockScope() as any

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
Expand Down
8 changes: 7 additions & 1 deletion packages/core/InteractableSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ export default class InteractableSet {
? this.selectorMap[target]
: target[this.scope.id]

targetMappings.splice(targetMappings.findIndex((m) => m.context === context), 1)
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)
})
}

Expand Down
23 changes: 22 additions & 1 deletion packages/core/Interaction.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :D

}

t.equal(interaction.pointerType, testType,
Expand All @@ -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()

Expand Down
6 changes: 6 additions & 0 deletions packages/core/Interaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,12 @@ export class Interaction<T extends ActionName = any> {
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

Expand Down
4 changes: 4 additions & 0 deletions packages/core/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,12 @@ export class Scope {
if (interaction.interactable === this) {
interaction.stop()
}
scope.interactions.signals.fire('destroy', { interaction })
interaction.destroy()
}

scope.interactions.list = []

scope.interactables.signals.fire('unset', { interactable: this })
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/interact/interact.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
4 changes: 4 additions & 0 deletions scripts/test.sh
Original file line number Diff line number Diff line change
@@ -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
Expand Down