fix: vscroll event mem leak causing unexpected behavior after filtering (#261)

- there was an event leak for the Virtual-Scroll which had the side effect of duplicating the scroll event everytime we were filtering, this indirectly caused weird behavior with the scroll and also items being skipped. I found the leak via the browser by investigating the events attached to the ul list element

Author: ghiscoding

packages/demo/src/options/options36.ts

@@ -7,10 +7,10 @@ export default class Example {
   mount() {
     const data1 = [];
     const data2 = [];
-    for (let i = 0; i < 25; i++) {
+    for (let i = 1; i <= 25; i++) {
       data1.push({ text: `Title ${i}`, value: i });
     }
-    for (let i = 0; i < 2000; i++) {
+    for (let i = 1; i <= 2000; i++) {
       data2.push({ text: `<i class="fa fa-star"></i> Task ${i}`, value: i });
     }
 

packages/multiple-select-vanilla/src/MultipleSelectInstance.ts

@@ -460,9 +460,7 @@ export class MultipleSelectInstance {
       offset = -1;
     }
 
-    if (this.options.virtualScroll && rows.length > Constants.BLOCK_ROWS * Constants.CLUSTER_BLOCKS) {
-      this.virtualScroll?.destroy();
-
+    if (rows.length > Constants.BLOCK_ROWS * Constants.CLUSTER_BLOCKS) {
       const dropVisible = this.dropElm.style.display !== 'none';
       if (!dropVisible) {
         this.dropElm.style.left = '-10000';
@@ -480,8 +478,9 @@ export class MultipleSelectInstance {
             this.updateDataStart = 0;
             this._currentHighlightIndex = 0;
           }
-          if (this.updateDataEnd > this.getDataLength()) {
-            this.updateDataEnd = this.getDataLength();
+          const dataLn = this.getDataLength();
+          if (this.updateDataEnd > dataLn) {
+            this.updateDataEnd = dataLn;
           }
 
           if (this.ulElm) {
@@ -495,18 +494,21 @@ export class MultipleSelectInstance {
       };
 
       if (this.ulElm) {
-        this.virtualScroll = new VirtualScroll({
-          rows,
-          scrollEl: this.ulElm,
-          contentEl: this.ulElm,
-          sanitizer: this.options.sanitizer,
-          callback: () => {
-            updateDataOffset();
-            this.events();
-          },
-        });
+        if (!this.virtualScroll) {
+          this.virtualScroll = new VirtualScroll({
+            rows,
+            scrollEl: this.ulElm,
+            contentEl: this.ulElm,
+            sanitizer: this.options.sanitizer,
+            callback: () => {
+              updateDataOffset();
+              this.events();
+            },
+          });
+        } else {
+          this.virtualScroll.reset(rows);
+        }
       }
-
       updateDataOffset();
 
       if (!dropVisible) {
@@ -521,7 +523,6 @@ export class MultipleSelectInstance {
       }
       this.updateDataStart = 0;
       this.updateDataEnd = this.updateData.length;
-      this.virtualScroll = null;
     }
 
     this.events();

packages/multiple-select-vanilla/src/services/virtual-scroll.ts

@@ -3,19 +3,19 @@ import type { HtmlStruct, VirtualCache, VirtualScrollOption } from '../interface
 import { convertItemRowToHtml, emptyElement } from '../utils';
 
 export class VirtualScroll {
-  cache: VirtualCache;
-  clusterRows?: number;
+  protected clusterRows?: number;
+  protected cache: VirtualCache;
+  protected scrollEl: HTMLElement;
+  protected blockHeight?: number;
+  protected clusterHeight?: number;
+  protected contentEl: HTMLElement;
+  protected parentEl: HTMLElement | null;
+  protected itemHeight?: number;
+  protected lastCluster: number;
+  protected scrollTop: number;
   dataStart!: number;
   dataEnd!: number;
   rows: HtmlStruct[];
-  scrollEl: HTMLElement;
-  blockHeight?: number;
-  clusterHeight?: number;
-  contentEl: HTMLElement;
-  parentEl: HTMLElement | null;
-  itemHeight?: number;
-  lastCluster: number;
-  scrollTop: number;
   destroy: () => void;
   callback: () => void;
   sanitizer?: (dirtyHtml: string) => string;
@@ -49,7 +49,14 @@ export class VirtualScroll {
     };
   }
 
-  initDOM(rows: HtmlStruct[]) {
+  reset(rows: HtmlStruct[]) {
+    this.lastCluster = 0;
+    this.cache = {} as any;
+    emptyElement(this.contentEl);
+    this.initDOM(rows);
+  }
+
+  protected initDOM(rows: HtmlStruct[]) {
     if (typeof this.clusterHeight === 'undefined') {
       this.cache.scrollTop = this.scrollEl.scrollTop;
       const firstRowElm = convertItemRowToHtml(rows[0]);
@@ -82,7 +89,7 @@ export class VirtualScroll {
     }
   }
 
-  getRowsHeight() {
+  protected getRowsHeight() {
     if (typeof this.itemHeight === 'undefined') {
       // make sure parent is not hidden before reading item list height
       const prevParentDisplay = this.parentEl?.style.display || '';
@@ -101,7 +108,7 @@ export class VirtualScroll {
     this.clusterHeight = this.blockHeight * Constants.CLUSTER_BLOCKS;
   }
 
-  getNum() {
+  protected getNum() {
     this.scrollTop = this.scrollEl.scrollTop;
     const blockSize = (this.clusterHeight || 0) - (this.blockHeight || 0);
     if (blockSize) {
@@ -110,7 +117,7 @@ export class VirtualScroll {
     return 0;
   }
 
-  initData(rows: HtmlStruct[], num: number) {
+  protected initData(rows: HtmlStruct[], num: number) {
     if (rows.length < Constants.BLOCK_ROWS) {
       return {
         topOffset: 0,
@@ -143,13 +150,13 @@ export class VirtualScroll {
     };
   }
 
-  checkChanges<T extends keyof VirtualCache>(type: T, value: VirtualCache[T]) {
+  protected checkChanges<T extends keyof VirtualCache>(type: T, value: VirtualCache[T]) {
     const changed = value !== this.cache[type];
     this.cache[type] = value;
     return changed;
   }
 
-  getExtra(className: string, height: number) {
+  protected getExtra(className: string, height: number) {
     const tag = document.createElement('li');
     tag.className = `virtual-scroll-${className}`;
     if (height) {

playwright/e2e/options36.spec.ts

@@ -9,49 +9,49 @@ test.describe('Options 36 - Infinite Scroll', () => {
 
     const ulElm1 = await page.locator('[data-test="select1"] .ms-drop ul');
     const liElms1 = await page.locator('[data-test="select1"] .ms-drop ul li');
-    await expect(liElms1.nth(0)).toContainText('Title 0');
+    await expect(liElms1.nth(0)).toContainText('Title 1');
     await liElms1.nth(0).click();
-    await expect(page.locator('[data-test=select1].ms-parent .ms-choice span')).toHaveText('Title 0');
+    await expect(page.locator('[data-test=select1].ms-parent .ms-choice span')).toHaveText('Title 1');
 
     // scroll near the end of the list
     await page.locator('[data-test="select1"].ms-parent').click();
     await ulElm1.evaluate(e => (e.scrollTop = e.scrollHeight - 10));
-    await page.locator('[data-test="select1"] .ms-drop label').filter({ hasText: 'Title 24' }).click();
+    await page.locator('[data-test="select1"] .ms-drop label').filter({ hasText: 'Title 25' }).click();
 
     // scroll completely to the end of the list & expect scrolling back to top
     await page.locator('[data-test="select1"].ms-parent').click();
     await ulElm1.evaluate(e => (e.scrollTop = e.scrollHeight));
     const firstTitleLoc = await page.locator('div[data-test=select1] .ms-drop li:nth-of-type(1)');
-    await expect(firstTitleLoc).toContainText('Title 0');
+    await expect(firstTitleLoc).toContainText('Title 1');
     await expect(firstTitleLoc).toHaveClass('hide-radio highlighted');
     await page.keyboard.press('Enter');
 
     // -- 2nd Select
     await page.locator('[data-test=select2].ms-parent').click();
     const ulElm2 = await page.locator('[data-test="select2"] .ms-drop ul');
     const liElms2 = await page.locator('[data-test="select2"] .ms-drop ul li');
-    await expect(await liElms2.nth(4).locator('span').innerHTML()).toBe('<i class="fa fa-star"></i> Task 4');
+    await expect(await liElms2.nth(4).locator('span').innerHTML()).toBe('<i class="fa fa-star"></i> Task 5');
     await liElms2.nth(4).click();
-    await expect(await liElms2.nth(5).locator('span').innerHTML()).toBe('<i class="fa fa-star"></i> Task 5');
+    await expect(await liElms2.nth(5).locator('span').innerHTML()).toBe('<i class="fa fa-star"></i> Task 6');
     await liElms2.nth(5).click();
-    await page.getByRole('button', { name: '4, 5' }).click();
+    await page.getByRole('button', { name: '5, 6' }).click();
 
     // scroll to the middle and click 1003
     await page.locator('[data-test="select2"].ms-parent').click();
     await ulElm2.evaluate(e => (e.scrollTop = e.scrollHeight / 2));
     await page.locator('[data-test="select2"] .ms-drop label').filter({ hasText: '1003' }).click();
-    await page.getByRole('button', { name: '4, 5, 1003' });
+    await page.getByRole('button', { name: '5, 6, 1003' });
 
     // scroll to near the end and select last 2 labels
     await ulElm2.evaluate(e => (e.scrollTop = e.scrollHeight - 300));
     await expect(await page.locator('[data-test="select2"] .ms-drop li[data-key=option_1995] label span').innerHTML()).toBe(
-      '<i class="fa fa-star"></i> Task 1995',
+      '<i class="fa fa-star"></i> Task 1996',
     );
     await expect(await page.locator('[data-test="select2"] .ms-drop li[data-key=option_1996] label span').innerHTML()).toBe(
-      '<i class="fa fa-star"></i> Task 1996',
+      '<i class="fa fa-star"></i> Task 1997',
     );
-    await page.locator('[data-test="select2"] .ms-drop label').filter({ hasText: '1995' }).click();
     await page.locator('[data-test="select2"] .ms-drop label').filter({ hasText: '1996' }).click();
+    await page.locator('[data-test="select2"] .ms-drop label').filter({ hasText: '1997' }).click();
     await page.getByRole('button', { name: '5 of 2000 selected' });
 
     // pressing arrow down until we reach the end will scroll back to top of the list
@@ -60,10 +60,10 @@ test.describe('Options 36 - Infinite Scroll', () => {
     page.keyboard.press('ArrowDown');
     await expect(await page.locator('[data-test="select2"] .ms-drop li[data-key=option_1999]')).toHaveClass('highlighted');
 
-    page.keyboard.press('ArrowDown'); // Task 0 (scrolled back to top)
+    page.keyboard.press('ArrowDown'); // Task 1 (scrolled back to top)
 
     const firstTaskLoc = await page.locator('div[data-test=select2] .ms-drop li:nth-of-type(1)');
-    await expect(firstTaskLoc).toContainText('Task 0');
+    await expect(firstTaskLoc).toContainText('Task 1');
     // await expect(await page.locator('[data-test="select2"] .ms-drop li[data-key=option_0]')).toHaveClass('highlighted');
   });
 });