Skip to content
69 changes: 69 additions & 0 deletions cypress/e2e/timer-cleanup.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/** cSpell:ignore vscomp */

// Tests for P4: pending setTimeout callbacks were not cleared on destroy(), so they could run
// against a destroyed instance. Timers now go through setManagedTimeout and are cleared in destroy().

describe('Hardening: managed timeouts cleared on destroy (P4)', () => {
const mountId = 'vs-p4-timers';

const mount = (win: Window) => {
const doc = win.document;
const existing = doc.getElementById(mountId);
if (existing) {
existing.remove();
}
const $ele = doc.createElement('div');
$ele.id = mountId;
doc.body.appendChild($ele);
// @ts-expect-error - VirtualSelect attached to window by the bundle
win.VirtualSelect.init({ ele: $ele, options: [{ label: 'A', value: 'a' }, { label: 'B', value: 'b' }] });
return $ele.virtualSelect;
};

it('cancels a pending managed timeout when the instance is destroyed', () => {
cy.visit('get-started');
// Use a synthetic clock so the negative assertion (timer must NOT fire) is deterministic.
// With a real-time cy.wait, CI timer-throttling could delay a still-pending 50ms callback
// past the wait window, making the test pass even if destroy() failed to clear it.
cy.clock();
cy.window().then((win) => {
// @ts-expect-error - test marker
win.__managedTimerFired = false;
const instance = mount(win);
instance.setManagedTimeout(() => {
// @ts-expect-error - test marker
win.__managedTimerFired = true;
}, 50);
instance.destroy();
});

// Advance well past the 50ms timeout; a cleared timer can never fire, a leaked one would.
cy.tick(150);
cy.window().then((win) => {
// @ts-expect-error - test marker
expect(win.__managedTimerFired, 'managed timeout must not fire after destroy').to.eq(false);
});
});
Comment thread
gnbm marked this conversation as resolved.

it('does not throw when destroying right after an option select (no regression)', () => {
cy.visit('get-started');
cy.window().then((win) => mount(win));

cy.get(`#${mountId}`).find('.vscomp-toggle-button').click();
cy.get(`#${mountId}`).find('.vscomp-option[data-value="a"]').click();

cy.window().then((win) => {
const $ele = win.document.getElementById(mountId);
// @ts-expect-error - instance back-reference
$ele.virtualSelect.destroy();
});

cy.wait(50);
cy.get('body').should('exist');
cy.window().then((win) => {
const $ele = win.document.getElementById(mountId);
// @ts-expect-error - instance back-reference
expect($ele.virtualSelect, 'instance reference cleared on destroy').to.eq(undefined);
});
});
});
2 changes: 1 addition & 1 deletion dist-archive/virtual-select-1.2.0.min.js

Large diffs are not rendered by default.

45 changes: 39 additions & 6 deletions dist/virtual-select.js
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,13 @@ class VirtualSelect {
} = Utils;
let groupName = '';
if (markSearchResults) {
/**
* Search input is regex-escaped (no ReDoS). The (?!([^<]+)?>) lookahead avoids inserting
* <mark> inside a tag; it relies on option labels being escaped via enableSecureText. When
* enableSecureText is off, labels are rendered as raw HTML by design (the consumer opts into
* this), so this highlight does not introduce an additional injection vector beyond the raw
* HTML the consumer already chose to render.
*/
searchRegex = new RegExp(`(${Utils.regexEscape(this.searchValue)})(?!([^<]+)?>)`, 'gi');
}
if (this.multiple) {
Expand Down Expand Up @@ -1296,7 +1303,7 @@ class VirtualSelect {
}

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.setSearchValue('');
this.focusSearchInput();
}, 0);
Expand Down Expand Up @@ -1357,7 +1364,7 @@ class VirtualSelect {
this.setOptionsPosition();
this.setOptionsTooltip();
if (document.activeElement !== this.$searchInput) {
setTimeout(() => {
this.setManagedTimeout(() => {
const focusedOption = DomUtils.getElementsBySelector('.focused', this.$dropboxContainer)[0];
if (focusedOption !== undefined) {
focusedOption.focus({
Expand Down Expand Up @@ -2809,8 +2816,9 @@ class VirtualSelect {
DomUtils.removeClass(this.$allWrappers, 'closed');
DomUtils.changeTabIndex(this.$allWrappers, 0);
if (!isSilent) {
// Force synchronous layout and style calculation
// Trigger reflow
// INTENTIONAL forced reflow (do not remove as a "no-op"): reading offsetHeight flushes
// the 'transition: none' set above so restoring the transition below does not animate the
// open from a stale layout. Scoped to a single element on open, so the cost is negligible.
this.$dropboxContainer.offsetHeight; // eslint-disable-line no-unused-expressions
// Restore transitions immediately after reflow
this.$dropboxContainer.style.transition = originalTransition;
Expand Down Expand Up @@ -3166,7 +3174,7 @@ class VirtualSelect {
}

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.renderOptions();
}, 0);
}
Expand Down Expand Up @@ -3302,7 +3310,7 @@ class VirtualSelect {
this.setValue(selectedValues);

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.renderOptions();
}, 0);
}
Expand Down Expand Up @@ -3538,6 +3546,28 @@ class VirtualSelect {
DomUtils.toggleClass(this.$allWrappers, 'has-error', hasError);
return !hasError;
}

/**
* setTimeout wrapper whose pending timers are tracked so they can be cleared on destroy().
* Prevents callbacks from running against a destroyed instance (stale DOM access / retention).
*/
setManagedTimeout(callback, delay) {
if (!this.managedTimeouts) {
this.managedTimeouts = new Set();
}
const id = setTimeout(() => {
this.managedTimeouts.delete(id);
callback();
}, delay);
this.managedTimeouts.add(id);
return id;
}
clearManagedTimeouts() {
if (this.managedTimeouts) {
this.managedTimeouts.forEach(id => clearTimeout(id));
this.managedTimeouts.clear();
}
}
destroy() {
const {
$ele
Expand All @@ -3560,6 +3590,9 @@ class VirtualSelect {
this.serverSearchTimeout = null;
}

// Clear any other pending timeouts so their callbacks don't run on a destroyed instance
this.clearManagedTimeouts();

/** Remove all event listeners to prevent memory leaks and ensure proper cleanup */
this.removeEvents();
if (this.hasDropboxWrapper) {
Expand Down
2 changes: 1 addition & 1 deletion dist/virtual-select.min.js

Large diffs are not rendered by default.

45 changes: 39 additions & 6 deletions docs/assets/virtual-select.js
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,13 @@ class VirtualSelect {
} = Utils;
let groupName = '';
if (markSearchResults) {
/**
* Search input is regex-escaped (no ReDoS). The (?!([^<]+)?>) lookahead avoids inserting
* <mark> inside a tag; it relies on option labels being escaped via enableSecureText. When
* enableSecureText is off, labels are rendered as raw HTML by design (the consumer opts into
* this), so this highlight does not introduce an additional injection vector beyond the raw
* HTML the consumer already chose to render.
*/
Comment thread
gnbm marked this conversation as resolved.
searchRegex = new RegExp(`(${Utils.regexEscape(this.searchValue)})(?!([^<]+)?>)`, 'gi');
}
if (this.multiple) {
Expand Down Expand Up @@ -1296,7 +1303,7 @@ class VirtualSelect {
}

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.setSearchValue('');
this.focusSearchInput();
}, 0);
Expand Down Expand Up @@ -1357,7 +1364,7 @@ class VirtualSelect {
this.setOptionsPosition();
this.setOptionsTooltip();
if (document.activeElement !== this.$searchInput) {
setTimeout(() => {
this.setManagedTimeout(() => {
const focusedOption = DomUtils.getElementsBySelector('.focused', this.$dropboxContainer)[0];
if (focusedOption !== undefined) {
focusedOption.focus({
Expand Down Expand Up @@ -2809,8 +2816,9 @@ class VirtualSelect {
DomUtils.removeClass(this.$allWrappers, 'closed');
DomUtils.changeTabIndex(this.$allWrappers, 0);
if (!isSilent) {
// Force synchronous layout and style calculation
// Trigger reflow
// INTENTIONAL forced reflow (do not remove as a "no-op"): reading offsetHeight flushes
// the 'transition: none' set above so restoring the transition below does not animate the
// open from a stale layout. Scoped to a single element on open, so the cost is negligible.
this.$dropboxContainer.offsetHeight; // eslint-disable-line no-unused-expressions
// Restore transitions immediately after reflow
this.$dropboxContainer.style.transition = originalTransition;
Expand Down Expand Up @@ -3166,7 +3174,7 @@ class VirtualSelect {
}

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.renderOptions();
}, 0);
}
Expand Down Expand Up @@ -3302,7 +3310,7 @@ class VirtualSelect {
this.setValue(selectedValues);

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.renderOptions();
}, 0);
}
Expand Down Expand Up @@ -3538,6 +3546,28 @@ class VirtualSelect {
DomUtils.toggleClass(this.$allWrappers, 'has-error', hasError);
return !hasError;
}

/**
* setTimeout wrapper whose pending timers are tracked so they can be cleared on destroy().
* Prevents callbacks from running against a destroyed instance (stale DOM access / retention).
*/
setManagedTimeout(callback, delay) {
Comment thread
gnbm marked this conversation as resolved.
if (!this.managedTimeouts) {
this.managedTimeouts = new Set();
}
const id = setTimeout(() => {
this.managedTimeouts.delete(id);
callback();
}, delay);
this.managedTimeouts.add(id);
return id;
}
clearManagedTimeouts() {
if (this.managedTimeouts) {
this.managedTimeouts.forEach(id => clearTimeout(id));
this.managedTimeouts.clear();
}
}
destroy() {
const {
$ele
Expand All @@ -3560,6 +3590,9 @@ class VirtualSelect {
this.serverSearchTimeout = null;
}

// Clear any other pending timeouts so their callbacks don't run on a destroyed instance
this.clearManagedTimeouts();

/** Remove all event listeners to prevent memory leaks and ensure proper cleanup */
this.removeEvents();
if (this.hasDropboxWrapper) {
Expand Down
2 changes: 1 addition & 1 deletion docs/assets/virtual-select.min.js

Large diffs are not rendered by default.

49 changes: 43 additions & 6 deletions src/virtual-select.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,13 @@ export class VirtualSelect {
let groupName = '';

if (markSearchResults) {
/**
* Search input is regex-escaped (no ReDoS). The (?!([^<]+)?>) lookahead avoids inserting
* <mark> inside a tag; it relies on option labels being escaped via enableSecureText. When
* enableSecureText is off, labels are rendered as raw HTML by design (the consumer opts into
* this), so this highlight does not introduce an additional injection vector beyond the raw
* HTML the consumer already chose to render.
*/
Comment thread
gnbm marked this conversation as resolved.
searchRegex = new RegExp(`(${Utils.regexEscape(this.searchValue)})(?!([^<]+)?>)`, 'gi');
}

Expand Down Expand Up @@ -848,7 +855,7 @@ export class VirtualSelect {
}

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.setSearchValue('');
this.focusSearchInput();
}, 0);
Expand Down Expand Up @@ -920,7 +927,7 @@ export class VirtualSelect {
this.setOptionsTooltip();

if (document.activeElement !== this.$searchInput) {
setTimeout(() => {
this.setManagedTimeout(() => {
const focusedOption = DomUtils.getElementsBySelector('.focused', this.$dropboxContainer)[0];
if (focusedOption !== undefined) {
focusedOption.focus({ preventScroll: true });
Expand Down Expand Up @@ -2571,8 +2578,9 @@ export class VirtualSelect {
DomUtils.changeTabIndex(this.$allWrappers, 0);

if (!isSilent) {
// Force synchronous layout and style calculation
// Trigger reflow
// INTENTIONAL forced reflow (do not remove as a "no-op"): reading offsetHeight flushes
// the 'transition: none' set above so restoring the transition below does not animate the
// open from a stale layout. Scoped to a single element on open, so the cost is negligible.
this.$dropboxContainer.offsetHeight; // eslint-disable-line no-unused-expressions
// Restore transitions immediately after reflow
this.$dropboxContainer.style.transition = originalTransition;
Expand Down Expand Up @@ -2995,7 +3003,7 @@ export class VirtualSelect {
}

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.renderOptions();
}, 0);
}
Expand Down Expand Up @@ -3162,7 +3170,7 @@ export class VirtualSelect {
this.setValue(selectedValues);

/** using setTimeout to fix the issue of dropbox getting closed on select */
setTimeout(() => {
this.setManagedTimeout(() => {
this.renderOptions();
}, 0);
}
Expand Down Expand Up @@ -3445,6 +3453,32 @@ export class VirtualSelect {
return !hasError;
}

/**
* setTimeout wrapper whose pending timers are tracked so they can be cleared on destroy().
* Prevents callbacks from running against a destroyed instance (stale DOM access / retention).
*/
setManagedTimeout(callback, delay) {
if (!this.managedTimeouts) {
this.managedTimeouts = new Set();
}

const id = setTimeout(() => {
this.managedTimeouts.delete(id);
callback();
}, delay);

this.managedTimeouts.add(id);

return id;
}

clearManagedTimeouts() {
if (this.managedTimeouts) {
this.managedTimeouts.forEach((id) => clearTimeout(id));
this.managedTimeouts.clear();
}
}

destroy() {
const { $ele } = this;
$ele.virtualSelect = undefined;
Expand All @@ -3465,6 +3499,9 @@ export class VirtualSelect {
this.serverSearchTimeout = null;
}

// Clear any other pending timeouts so their callbacks don't run on a destroyed instance
this.clearManagedTimeouts();

/** Remove all event listeners to prevent memory leaks and ensure proper cleanup */
this.removeEvents();

Expand Down