perf: throttle global resize handler + guard instance-less wrappers by gnbm · Pull Request #472 · sa-si-dev/virtual-select

gnbm · 2026-06-03T22:05:46Z

Problem

static onResizeMethod() {
  document.querySelectorAll('.vscomp-ele-wrapper').forEach(($ele) => {
    $ele.parentElement.virtualSelect.onResize(); // -> setOptionsContainerHeight(true)
  });
}
window.addEventListener('resize', VirtualSelect.onResizeMethod);

The listener is unthrottled, so during a resize drag it runs full-document queries + per-instance height recomputation on every tick — O(instances) layout work per event.
It assumes every .vscomp-ele-wrapper's parent has a .virtualSelect and throws (Cannot read properties of undefined) if one is mid-teardown.

Fix

Add Utils.throttle() and wrap the resize listener at ~100 ms, so per-instance work runs at most ~10×/sec.
Utils.throttle() preserves the caller's this (invokes the callback via callback.apply(...) on both leading and trailing edges), keeping it safe as a general-purpose utility — addresses code-review feedback.
Guard onResizeMethod so wrappers without an instance are skipped.

No-regression note: closed instances are still updated. Opening a dropdown does not recompute the options-container height (only render-time and resize do), so skipping closed instances could leave a stale height on reopen. Throttling delivers the perf win without that risk; skipping-closed would require recompute-on-open and is left as a possible follow-up.

Verification

New spec cypress/e2e/perf-resize-throttle.cy.ts:

throttling: 10 rapid resize events → onResize runs >0 and <10 times;
no-regression: an open dropdown stays open/functional after a resize;
guard: an orphan .vscomp-ele-wrapper (no instance) doesn't throw on resize.

Other Validations:

Ran npm run validate for static code validation:
npm run build: succeeds; dist/docs rebuilt
All tests passed (ran npm run test ):

…wrappers (P1) The window 'resize' listener ran onResizeMethod on every tick, which queried all .vscomp-ele-wrapper elements and recomputed each instance's options-container height - O(instances) layout work per resize event during a drag. It also assumed every wrapper's parent had a .virtualSelect and threw otherwise. - Add Utils.throttle and wrap the resize listener (~100ms) so the per-instance work runs at most ~10x/sec. - Guard onResizeMethod against wrappers whose instance is missing (teardown races). Closed instances are still updated (opening does not recompute height, so skipping them could leave a stale height on reopen) - throttling delivers the win without that regression risk. Adds cypress spec perf-resize-throttle.cy.ts (throttling, no-regression, guard).

Fix lint issues

Add JSDoc/type annotations to Utils.throttle (timeout and lastArgs) and annotate the throttled function parameter. Replace callback.apply(this, lastArgs) with callback(...lastArgs) for simpler invocation and improved type-safety. Updated the built/minified distribution and docs asset files accordingly.

Copilot

Pull request overview

This PR improves resize-time performance and resilience by throttling VirtualSelect’s global window.resize handler and guarding against .vscomp-ele-wrapper elements whose instances are missing (e.g., during teardown). It also adds a Cypress regression spec and rebuilds the distributed/docs bundles.

Changes:

Throttle the global resize listener to ~100ms and skip wrappers without an attached virtualSelect instance.
Add Utils.throttle() helper to support rate-limiting high-frequency handlers.
Add Cypress E2E coverage for throttling behavior, no-regression on open dropdowns, and teardown-guard behavior.

Reviewed changes

Copilot reviewed 4 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/virtual-select.js	Adds instance guard in `onResizeMethod` and wraps the resize listener with `Utils.throttle(..., 100)`.
src/utils/utils.js	Introduces `Utils.throttle()` utility used by the global resize handler.
cypress/e2e/perf-resize-throttle.cy.ts	Adds E2E tests for throttling, regression (dropdown remains functional), and orphan-wrapper guard.
dist/virtual-select.js	Rebuilt bundle reflecting the new throttle + guard behavior.
dist/virtual-select.min.js	Rebuilt minified bundle reflecting the new throttle + guard behavior.
docs/assets/virtual-select.js	Rebuilt docs bundle reflecting the new throttle + guard behavior.
docs/assets/virtual-select.min.js	Rebuilt minified docs bundle reflecting the new throttle + guard behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 4 out of 8 changed files in this pull request and generated 1 comment.

@this

Invoke the throttled callback via callback.apply(lastThis, lastArgs) on both leading and trailing edges so instance methods used as throttled event handlers run with the expected 'this'. Add @this annotation to satisfy tsc. Regenerate dist/docs bundles.

Copilot

Pull request overview

Copilot reviewed 4 out of 8 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 8 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 8 changed files in this pull request and generated 1 comment.

…leanup gaps) (#468) * fix: remove document click listener correctly to prevent memory leak onDocumentClick is added to `document` in the capture phase (addEvent(document, 'click', 'onDocumentClick', true)), but removeEvent called removeEventListener without the capture flag. removeEventListener only unregisters a listener when its capture flag matches the one used at registration, so the capture-phase document listener was never removed. As a result, every render/destroy cycle (e.g. repeatedly opening and closing the dropdown) left a new onDocumentClick listener bound to `document`. Each leaked listener retains the VirtualSelect instance and its detached dropbox DOM subtree, so event-listener and DOM-node counts grow without bound and are never reclaimed by GC. Fix: thread the capture flag through DomUtils.removeEvent and the VirtualSelect.removeEvent wrapper, and pass capture: true when removing the document click listener so add and remove match. * fix: ensure cleanup runs in inline mode and clear retained references on destroy Two additional memory-leak hardening fixes: 1. Install the self-destroy MutationObserver in every mode, not only when hasDropboxWrapper is true. Previously, inline-mode dropdowns had no observer, so removing the host element from the DOM without calling destroy() left all addEvents() listeners attached - including the capture-phase document click listener that retains the instance and its detached DOM. removeMutationObserver now disconnects based on the observer's existence. 2. On destroy(), clear the $dropboxWrapper.virtualSelect back-reference (set in setEleProps) before detaching the wrapper, and reset this.events. This breaks the instance <-> detached-DOM reference cycle so the instance and its DOM can be reclaimed. * fix: limit mutation observer to popup mode to avoid per-instance perf penalty Reverts the all-modes MutationObserver install. In the default dropboxWrapper:'self' mode every instance was registering a body-wide subtree observer, adding O(instances x mutations) overhead on pages with many selects. The reported document-click listener leak is fixed by the capture-flag correction in removeEvents(); the observer stays scoped to popup mode as before.

The branch committed the unminified dist/docs sources and the dist-archive copy with the security (S2 customData escaping, S3 classNames sanitization) and perf (P1 throttled resize) fixes, but the primary distributed bundles (dist/virtual-select.min.js and docs/assets/virtual-select.min.js) were left stale - consumers loading them got none of the fixes. Regenerated via 'npm run build' so all three min bundles share an identical body and match the source.

Copilot

Pull request overview

Copilot reviewed 5 out of 9 changed files in this pull request and generated 2 comments.

…review) The throttled wrapper passed inline to addEventListener was anonymous, so the resize listener could no longer be detached via removeEventListener (the prior VirtualSelect.onResizeMethod reference was removable). Store the throttled function on VirtualSelect.onResizeThrottled and register that reference instead, restoring removability for future cleanup. Declared as a typed static field so it passes tsc (checkJs + strict).

Copilot

Pull request overview

Copilot reviewed 5 out of 9 changed files in this pull request and generated 2 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

This reverts commit 98c9649.

Copilot

Pull request overview

Copilot reviewed 5 out of 9 changed files in this pull request and generated 2 comments.

@ts-nocheck

…Copilot review) The static class-field declaration provided no type-safety benefit (the file is // @ts-nocheck, so tsc never checks it) and used ES2022 class-field syntax not present anywhere else in the codebase. The property is assigned at module init time via VirtualSelect.onResizeThrottled = ..., which is sufficient to expose a stable, removable reference. Runtime behaviour is unchanged.

Copilot

Pull request overview

Copilot reviewed 5 out of 9 changed files in this pull request and generated 2 comments.

…iew) throttle retained lastArgs/lastThis (e.g. a DOM Event passed to a resize handler) until the next call or indefinitely if never called again. Clear them after both the leading and trailing invocations so large arguments aren't held, matching the canonical underscore/lodash throttle. Behaviour is unchanged: both paths reassign lastArgs/lastThis at the top of every call before use.

Copilot

Pull request overview

Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.

os-davidlourenco

LGTM

gnbm mentioned this pull request Jun 3, 2026

chore: clear pending timers on destroy + clarify reflow/highlight intent #473

Merged

gnbm added the chore label Jun 3, 2026

gnbm added 2 commits June 4, 2026 00:06

Update utils.js

9d7e7e9

Fix lint issues

gnbm requested a review from Copilot June 6, 2026 13:47

Copilot started reviewing on behalf of gnbm June 6, 2026 13:47 View session

Copilot AI reviewed Jun 6, 2026

View reviewed changes

Comment thread src/utils/utils.js

Update virtual-select-1.2.0.min.js

d807b53

gnbm requested a review from Copilot June 6, 2026 15:14

Copilot started reviewing on behalf of gnbm June 6, 2026 15:14 View session

Copilot AI reviewed Jun 6, 2026

View reviewed changes

Comment thread src/utils/utils.js

gnbm requested a review from Copilot June 7, 2026 22:37

Copilot started reviewing on behalf of gnbm June 7, 2026 22:37 View session

Copilot AI reviewed Jun 7, 2026

View reviewed changes

gnbm requested a review from Copilot June 7, 2026 22:49

Copilot started reviewing on behalf of gnbm June 7, 2026 22:49 View session

Copilot AI reviewed Jun 7, 2026

View reviewed changes

gnbm requested a review from Copilot June 7, 2026 22:53

Copilot started reviewing on behalf of gnbm June 7, 2026 22:53 View session

Copilot AI reviewed Jun 7, 2026

View reviewed changes

Comment thread docs/assets/virtual-select.js

gnbm and others added 3 commits June 8, 2026 12:13

Merge branch 'master' into perf/throttle-resize-handler

0a489c9

gnbm requested a review from Copilot June 8, 2026 12:10

Copilot started reviewing on behalf of gnbm June 8, 2026 12:10 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Comment thread src/virtual-select.js Outdated

Comment thread docs/assets/virtual-select.js

gnbm requested a review from Copilot June 8, 2026 12:23

Copilot started reviewing on behalf of gnbm June 8, 2026 12:23 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Comment thread docs/assets/virtual-select.js

Comment thread src/virtual-select.js Outdated

gnbm and others added 2 commits June 8, 2026 13:36

Potential fix for pull request finding

98c9649

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Revert "Potential fix for pull request finding"

78bce11

This reverts commit 98c9649.

gnbm requested a review from Copilot June 8, 2026 12:37

Copilot started reviewing on behalf of gnbm June 8, 2026 12:37 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Comment thread src/virtual-select.js Outdated

Comment thread docs/assets/virtual-select.js

gnbm requested a review from Copilot June 8, 2026 12:50

Copilot started reviewing on behalf of gnbm June 8, 2026 12:50 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Comment thread src/utils/utils.js

Comment thread docs/assets/virtual-select.js

gnbm requested a review from Copilot June 8, 2026 12:58

Copilot started reviewing on behalf of gnbm June 8, 2026 12:58 View session

gnbm marked this pull request as ready for review June 8, 2026 12:58

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Comment thread docs/assets/virtual-select.js

os-davidlourenco approved these changes Jun 8, 2026

View reviewed changes

gnbm merged commit 6082f1c into security/sanitize-classnames Jun 8, 2026
1 check passed

Conversation

gnbm commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

os-davidlourenco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gnbm commented Jun 3, 2026 •

edited

Loading