fix(core): potential event binding leaks not all removed when disposing

examples/webpack-demo-vanilla-bundle/src/app.ts

@@ -108,6 +108,10 @@ export class App {
         };
       }
 
+      // first dispose binding from previous page
+      this.renderer.dispose();
+
+      // then load the new View
       this.renderer.loadView(require(`${mapRoute.moduleId}.html`));
       if (viewModel?.attached && this.renderer.className) {
         this.viewModelObj[this.renderer.className] = viewModel;

examples/webpack-demo-vanilla-bundle/src/renderer.ts

@@ -21,9 +21,12 @@ export class Renderer {
   }
 
   dispose() {
-    for (const observer of this._observers) {
-      observer.unbindAll();
+    let observer = this._observers.pop();
+    while (observer) {
+      observer.dispose();
+      observer = this._observers.pop();
     }
+    this._observers = [];
   }
 
   getModuleClassName(module: any): string {

packages/binding/src/__tests__/binding.helper.spec.ts

@@ -47,6 +47,10 @@ describe('Binding Helper', () => {
     const mockEvent = new CustomEvent('change', { bubbles: true, detail: { target: { value: 'Jane' } } });
     elm.dispatchEvent(mockEvent);
 
+    // remove the UID before comparing or else it will always fail
+    delete helper.observers[0].boundedEventWithListeners[0].uid;
+    delete helper.observers[1].boundedEventWithListeners[0].uid;
+
     expect(helper.observers.length).toBe(2);
     expect(JSON.stringify(helper.observers[0])).toEqual(JSON.stringify(helper.observers[1]));
     expect(helper.querySelectorPrefix).toBe('');

packages/binding/src/__tests__/binding.service.spec.ts

@@ -67,15 +67,16 @@ describe('Binding Service', () => {
   });
 
   it('should unbind an event from an element', () => {
-    const mockElm = { removeEventListener: jest.fn() } as unknown as HTMLElement;
+    const mockElm = { addEventListener: jest.fn(), removeEventListener: jest.fn() } as unknown as HTMLElement;
     const mockCallback = jest.fn();
     const removeEventSpy = jest.spyOn(mockElm, 'removeEventListener');
     const mockObj = { name: 'John', age: 20 };
     const elm = document.createElement('input');
     div.appendChild(elm);
 
     service = new BindingService({ variable: mockObj, property: 'name' });
-    service.unbind(mockElm, 'click', mockCallback, false);
+    service.bind(mockElm, 'value', 'keyup');
+    service.unbind(mockElm, 'click', mockCallback, false, service.boundedEventWithListeners[0].uid);
 
     expect(service.property).toBe('name');
     expect(removeEventSpy).toHaveBeenCalledWith('click', mockCallback, false);

packages/binding/src/binding.service.ts

@@ -1,3 +1,4 @@
+/* eslint-disable no-bitwise */
 import * as DOMPurify_ from 'dompurify';
 const DOMPurify = DOMPurify_; // patch to fix rollup to work
 
@@ -15,23 +16,30 @@ export interface ElementBindingWithListener extends ElementBinding {
   listener: (val: any) => any;
 }
 
+export interface BoundedEventWithListener {
+  element: Element;
+  eventName: string;
+  listener: EventListenerOrEventListenerObject;
+  uid: string;
+}
+
 /**
  * Create 2 way Bindings for any variable that are primitive or object types, when it's an object type it will watch for property changes
  * The following 2 articles helped in building this service:
  *   1- https://blog.jeremylikness.com/blog/client-side-javascript-databinding-without-a-framework/
  *   2- https://www.wintellect.com/data-binding-pure-javascript/
  */
 export class BindingService {
-  _value: any = null;
-  _binding: Binding;
-  _boundedEventWithListeners: { element: Element; eventName: string; listener: EventListenerOrEventListenerObject; }[] = [];
-  _property: string;
-  elementBindings: Array<ElementBinding | ElementBindingWithListener> = [];
+  protected _value: any = null;
+  protected _binding: Binding;
+  protected _property: string;
+  protected _boundedEventWithListeners: BoundedEventWithListener[] = [];
+  protected _elementBindings: Array<ElementBinding | ElementBindingWithListener> = [];
 
   constructor(binding: Binding) {
     this._binding = binding;
     this._property = binding.property || '';
-    this.elementBindings = [];
+    this._elementBindings = [];
     if (binding.property && binding.variable && (binding.variable.hasOwnProperty(binding.property) || binding.property in binding.variable)) {
       this._value = typeof binding.variable[binding.property] === 'string' ? this.sanitizeText(binding.variable[binding.property]) : binding.variable[binding.property];
     } else {
@@ -46,14 +54,22 @@ export class BindingService {
     }
   }
 
+  get boundedEventWithListeners(): BoundedEventWithListener[] {
+    return this._boundedEventWithListeners;
+  }
+
+  get elementBindings(): Array<ElementBinding | ElementBindingWithListener> {
+    return this._elementBindings;
+  }
+
   get property() {
     return this._property;
   }
 
   dispose() {
     this.unbindAll();
     this._boundedEventWithListeners = [];
-    this.elementBindings = [];
+    this._elementBindings = [];
   }
 
   valueGetter() {
@@ -62,8 +78,8 @@ export class BindingService {
 
   valueSetter(val: any) {
     this._value = typeof val === 'string' ? this.sanitizeText(val) : val;
-    if (Array.isArray(this.elementBindings)) {
-      for (const binding of this.elementBindings) {
+    if (Array.isArray(this._elementBindings)) {
+      for (const binding of this._elementBindings) {
         if (binding?.element && binding?.attribute) {
           (binding.element as any)[binding.attribute] = typeof val === 'string' ? this.sanitizeText(val) : val;
         }
@@ -90,17 +106,23 @@ export class BindingService {
   }
 
   /** Unbind (remove) an element event listener */
-  unbind(element: Element | null, eventName: string, listener: EventListenerOrEventListenerObject, options?: boolean | EventListenerOptions) {
+  unbind(element: Element | null, eventName: string, listener: EventListenerOrEventListenerObject, options?: boolean | EventListenerOptions, eventUid?: string) {
     if (element) {
       element.removeEventListener(eventName, listener, options);
+      const eventIdx = this._boundedEventWithListeners.findIndex(be => be.uid === eventUid);
+      if (eventIdx >= 0) {
+        this._boundedEventWithListeners.splice(eventIdx, 1);
+      }
     }
   }
 
   /** Unbind All (remove) bounded elements with listeners */
   unbindAll() {
-    for (const boundedEvent of this._boundedEventWithListeners) {
-      const { element, eventName, listener } = boundedEvent;
-      this.unbind(element, eventName, listener);
+    let boundedEvent = this._boundedEventWithListeners.pop();
+    while (boundedEvent) {
+      const { element, eventName, listener, uid } = boundedEvent;
+      this.unbind(element, eventName, listener, undefined, uid);
+      boundedEvent = this._boundedEventWithListeners.pop();
     }
     this._boundedEventWithListeners = [];
   }
@@ -130,13 +152,22 @@ export class BindingService {
         (binding as ElementBindingWithListener).event = eventName;
         (binding as ElementBindingWithListener).listener = listener;
         element.addEventListener(eventName, listener);
-        this._boundedEventWithListeners.push({ element, eventName, listener });
+        this._boundedEventWithListeners.push({ element, eventName, listener, uid: this.generateUuidV4() });
       }
-      this.elementBindings.push(binding);
+      this._elementBindings.push(binding);
       (element as any)[attribute] = typeof this._value === 'string' ? this.sanitizeText(this._value) : this._value;
     }
   }
 
+  /** Generate a UUID version 4 RFC compliant */
+  protected generateUuidV4() {
+    return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
+      const r = Math.random() * 16 | 0;
+      const v = c === 'x' ? r : (r & 0x3 | 0x8);
+      return v.toString(16);
+    });
+  }
+
   protected sanitizeText(dirtyText: string): string {
     return (DOMPurify?.sanitize) ? DOMPurify.sanitize(dirtyText, {}) : dirtyText;
   }