Datepicker creating detached elements

[joel-szakall]

Overview of the problem

Buefy version: 0.9.23 (I think it was also happening in 0.9.14 as well)
Vuejs version: 2.6.14
OS/Browser: Edge 111.0.1661.44 (64-bit) on Windows 10; Chrome

Description

Event listeners are not being unbound on Datepicker child components after it is destroyed, causing a memory leak due to detached elements that are not cleaned up by the garbage collector.

Steps to reproduce

1. Use Datepicker component and add conditional rendering using "v-if"
2. Allow Datepicker to mount/render on page (ie. v-if=true)
3. Set condition to false thus destroying the Datepicker component
4. Observe detached elements via the Chrome/Edge Dev Tools, under the "Detached Elements" tool.

Expected behavior

All event listeners and DOM elements cleaned up/destroyed and garbage collected after Datepicker component is destroyed.

Actual behavior

After being destroyed, the Datepicker component is removed from the DOM, but its elements are still being referenced in code, thus retaining the detached elements in memory. I suspect the culprit to be located in the bind() and unbind() functions in the 'trapFocus.js' directive. It appears unbind() is not being run for 'onKeyUp' event listeners for some/all elements within the child components of the Datepicker, and these events are holding onto references for the days of the month and other keyboard-navigable elements (I saw some on anchor elements). I think the elements with 'onKeyUp' are being deleted from the DOM before unbind() can be performed on them. Every parent component (if the Datepicker is nested inside other components) is also being held onto in memory due to this issue.

At times, these detached elements persist through page reloads as well; closing and reopening the app in a new window/tab, or navigating to a different address and then navigating back to the app clears the detached elements.

[wesdevpro]

@joel-szakall Thank you for letting me know about this issue. If you could please send me a code pen that would be great. I'll get to work on this when I have some free time. Thanks

[kikuomax]

@joel-szakall @wesdevpro I have investigated the problem on Edge on macOS. And I have found Datepicker temporarily leaks, but the leaks should not accumulate. You can confirm it by taking the following steps,

1. Open "Detached Elements" tool
2. Show Datepicker by making v-if="true"
3. Hide Datepicker by making v-if="false"
4. Press "Get Detached Elements" and see a detached Datepicker
5. Press "Collect garbage"
6. Press "Get Detached Elements" and see the detached Datepicker remaining
7. Show Datepicker by making v-if="true"
8. Press "Get Detached Elements" and see the detached Datepicker and some other elements
9. Press "Collect garbage"
10. Press "Get Detached Elements" and see the list cleared

[kikuomax]

@joel-szakall @wesdevpro Here is what happens under the hood:

trapFocus directive remembers the last "keydown" event handler in onKeyDown:

buefy/src/directives/trapFocus.js

Line 21 in d3bd40d

let onKeyDown

2. Show Datepicker by making v-if="true"

This step stores a new "keydown" event handler in onKeyDown and attaches it to the element (Datepicker):

buefy/src/directives/trapFocus.js

Lines 29 to 45 in d3bd40d

	onKeyDown = (event) => {
	// Need to get focusable each time since it can change between key events
	// ex. changing month in a datepicker
	focusable = findFocusable(el)
	focusableProg = findFocusable(el, true)
	const firstFocusable = focusable[0]
	const lastFocusable = focusable[focusable.length - 1]
	if (event.target === firstFocusable && event.shiftKey && event.key === 'Tab') {
	event.preventDefault()
	lastFocusable.focus()
	} else if ((event.target === lastFocusable || Array.from(focusableProg).indexOf(event.target) >= 0) && !event.shiftKey && event.key === 'Tab') {
	event.preventDefault()
	firstFocusable.focus()
	}
	}
	el.addEventListener('keydown', onKeyDown)

onKeyDown holds the element as it refers to the element in it.

3. Hide Datepicker by making v-if="false"

This step removes onKeyDown from the element but never resets onKeyDown. So onKeyDown stays holding the element.

buefy/src/directives/trapFocus.js

Lines 50 to 52 in d3bd40d

	const unbind = (el) => {
	el.removeEventListener('keydown', onKeyDown)
	}

This is why you see the Datepicker remaining at Step 6.

6. Press "Get Detached Elements" and see the detached Datepicker remaining

7. Show Datepicker by making v-if="true"

This overwrites onKeyDown with a new "keydown" event handler. Therefore, the old event handler is freed and the detached Datepicker becomes "garbage collectable". This is why you see nothing remaining at Step 10:

10. Press "Get Detached Elements" and see the list cleared

[kikuomax]

Having two or more v-trap-focus directives might cause a problem, though, this is another issue and needs more investigation.

[wesdevpro]

@kikuomax It looks like I have some more learning to do 🤣