simplify dom observer api and prevent memory leak

packages/overlayscrollbars/src/observers/domObserver.ts

@@ -20,7 +20,7 @@ interface DOMTargetObserverOptions extends DOMObserverOptionsBase {
   _ignoreTargetChange?: DOMObserverIgnoreTargetChange; // a function which will prevent marking certain attributes as changed if it returns true
 }
 
-type ContentChangeArrayItem = [string?, string?, boolean?] | null | undefined;
+type ContentChangeArrayItem = [string?, string?] | null | undefined;
 
 export type DOMObserverEventContentChange = Array<ContentChangeArrayItem> | false | null | undefined;
 
@@ -56,29 +56,22 @@ export interface DOMObserver {
  * @param callback Callback which is called if one of the elements emits the corresponding event.
  * @returns A object which contains a set of helper functions to destroy and update the observation of elements.
  */
-const createEventContentChange = (target: Element, eventContentChange: DOMObserverEventContentChange, callback: (...args: any) => any) => {
+const createEventContentChange = (target: Element, callback: (...args: any) => any, eventContentChange?: DOMObserverEventContentChange) => {
-  let eventSet: Set<() => any> | undefined;
+  let map: WeakMap<Node, [string, () => any]> | undefined; // weak map to prevent memory leak for detached elements
-  let onceSet: WeakMap<Node, 0> | undefined; // use WeakMap instead of WeakSet because of IE11 support
   let destroyed = false;
   const _destroy = () => {
     destroyed = true;
-    if (eventSet) {
-      eventSet.forEach((offFn) => {
-        offFn();
-      });
-      eventSet.clear();
-    }
   };
   const _updateElements = (getElements?: (selector: string) => Node[]) => {
-    if (eventSet && onceSet && eventContentChange) {
+    if (eventContentChange) {
-      const eventElmList = eventContentChange.reduce<Array<[Node[], string, boolean]>>((arr, item) => {
+      const eventElmList = eventContentChange.reduce<Array<[Node[], string]>>((arr, item) => {
         if (item) {
           const selector = item[0];
           const eventNames = item[1];
           const elements = eventNames && selector && (getElements ? getElements(selector) : find(selector, target));
 
           if (elements && elements.length && eventNames && isString(eventNames)) {
-            push(arr, [elements, eventNames.trim(), !!item[2]], true);
+            push(arr, [elements, eventNames.trim()], true);
           }
         }
         return arr;
@@ -87,31 +80,34 @@ const createEventContentChange = (target: Element, eventContentChange: DOMObserv
       each(eventElmList, (item) =>
         each(item[0], (elm) => {
           const eventNames = item[1];
-          const once = item[2];
+          const entry = map!.get(elm);
 
-          if (once && !onceSet!.has(elm)) {
+          if (entry) {
-            onceSet!.set(elm, 0);
+            const entryEventNames = entry[0];
-            on(
+            const entryOff = entry[1];
-              elm,
+
-              eventNames,
+            // in case an already registered element is registered again, unregister the previous events
-              (event) => {
+            if (entryEventNames === eventNames) {
-                if (!destroyed) {
+              entryOff();
-                  callback(event);
+            }
-                }
-              },
-              { _once: once }
-            );
-          } else {
-            eventSet!.add(on(elm, eventNames, callback));
           }
+
+          const off = on(elm, eventNames, (event: Event) => {
+            if (destroyed) {
+              off();
+              map!.delete(elm);
+            } else {
+              callback(event);
+            }
+          });
+          map!.set(elm, [eventNames, off]);
         })
       );
     }
   };
 
   if (eventContentChange) {
-    eventSet = eventSet || new Set();
+    map = new WeakMap();
-    onceSet = onceSet || new WeakMap();
     _updateElements();
   }
 
@@ -147,7 +143,6 @@ export const createDOMObserver = <ContentObserver extends boolean>(
   } = (options as DOMContentObserverOptions & DOMTargetObserverOptions) || {};
   const { _destroy: destroyEventContentChange, _updateElements: updateEventContentChangeElements } = createEventContentChange(
     target,
-    isContentObserver && _eventContentChange,
     debounce(
       () => {
         if (isConnected) {
@@ -155,7 +150,8 @@ export const createDOMObserver = <ContentObserver extends boolean>(
         }
       },
       { _timeout: 33, _maxDelay: 99 }
-    )
+    ),
+    _eventContentChange
   );
 
   // MutationObserver

packages/overlayscrollbars/src/options.ts

@@ -34,7 +34,7 @@ export interface OSOptions {
   resize: ResizeBehavior;
   paddingAbsolute: boolean;
   updating: {
-    elementEvents: Array<[string, string, boolean?]> | null;
+    elementEvents: Array<[string, string]> | null;
     attributes: string[] | null;
     debounce: number | [number, number] | null;
   };
@@ -137,7 +137,7 @@ const defaultOptionsWithTemplate: OptionsWithOptionsTemplate<OSOptions> = {
   resize: ['none', resizeAllowedValues], // none || both  || horizontal || vertical || n || b || h || v
   paddingAbsolute: booleanFalseTemplate, // true || false
   updating: {
-    elementEvents: [[['img', 'load', true]], arrayNullValues], // array of tuples || null
+    elementEvents: [[['img', 'load']], arrayNullValues], // array of tuples || null
     attributes: [null, arrayNullValues],
     debounce: [
       [0, 33],

packages/overlayscrollbars/tests/browser/observers/domObserver/index.browser.ts

@@ -58,7 +58,7 @@ const startBtn: HTMLButtonElement | null = document.querySelector('#start');
 const hostSelector = '.host';
 const ignorePrefix = 'ignore';
 const attrs = ['id', 'class', 'style', 'open'];
-const contentChangeArr: Array<[string?, string?, boolean?]> = [['img', 'load', true]];
+const contentChange: Array<[string?, string?]> = [['img', 'load']];
 const domTargetObserverObservations: DOMTargetObserverResult[] = [];
 const domContentObserverObservations: DOMContentObserverResult[] = [];
 
@@ -106,7 +106,7 @@ const targetDomObserver = createDOMObserver(
   }
 );
 
-const createContentDomOserver = (eventContentChange: Array<[string?, string?, boolean?] | null | undefined>) => {
+const createContentDomOserver = (eventContentChange: Array<[string?, string?] | null | undefined>) => {
   return createDOMObserver(
     trargetContentElm!,
     true,
@@ -147,7 +147,7 @@ const createContentDomOserver = (eventContentChange: Array<[string?, string?, bo
   );
 };
 
-let contentDomObserver = createContentDomOserver(contentChangeArr);
+let contentDomObserver = createContentDomOserver(contentChange);
 
 const getTotalObservations = () => domTargetObserverObservations.length + domContentObserverObservations.length;
 const getLast = <T>(arr: T[], indexFromLast = 0): T => arr[arr.length - 1 - indexFromLast] || ({} as T);
@@ -424,7 +424,7 @@ const addRemoveImgElmsFn = async () => {
   await addMultiple();
 
   // remove load event from image test
-  const addChanged = async (newEventContentChange: Array<[string?, string?, boolean?] | null | undefined>) => {
+  const addChanged = async (newEventContentChange: Array<[string?, string?] | null | undefined>) => {
     contentDomObserver._destroy();
     contentDomObserver = createContentDomOserver(newEventContentChange);
 
@@ -446,7 +446,7 @@ const addRemoveImgElmsFn = async () => {
     });
 
     contentDomObserver._destroy();
-    contentDomObserver = createContentDomOserver(contentChangeArr);
+    contentDomObserver = createContentDomOserver(contentChange);
   };
 
   await addChanged([['img', 'something'], ['img', 'something2'], ['img', ''], ['img', undefined], ['', ''], [undefined, undefined], null, undefined]);
@@ -512,7 +512,7 @@ const addRemoveTransitionElmsFn = async () => {
 
     await startTransition(elm, expectTransitionEndContentChange && true);
     contentDomObserver._destroy();
-    contentDomObserver = createContentDomOserver(contentChangeArr);
+    contentDomObserver = createContentDomOserver(contentChange);
     await startTransition(elm, expectTransitionEndContentChange && false);
 
     removeElements(elm);
@@ -523,7 +523,7 @@ const addRemoveTransitionElmsFn = async () => {
   await add(false);
 
   contentDomObserver._destroy();
-  contentDomObserver = createContentDomOserver(contentChangeArr.concat([['.transition', 'transitionend']]));
+  contentDomObserver = createContentDomOserver(contentChange.concat([['.transition', 'transitionend']]));
 
   await add(true);
 };