Fix/memory leak (#4009)

* fix: memory leak issues * fix: `Dom.Event.off` error while passing unexist event * test: Add test case for `Dom.Event.off` bug --------- Co-authored-by: cluezhang <[email protected]>
antvis · Nov 6, 2023 · ffbc7ed · ffbc7ed
1 parent 339cade
commit ffbc7ed
Show file tree

Hide file tree

Showing 10 changed files with 67 additions and 15 deletions.
diff --git a/packages/x6-common/src/__tests__/dom/event.test.ts b/packages/x6-common/src/__tests__/dom/event.test.ts
@@ -294,10 +294,16 @@ describe('EventDom', () => {
         expect(() => div.off('click', false)).not.toThrowError()
       })
 
-      it('should do noting for elem which do not bind any events', () => {
+      it('should do nothing for elem which do not bind any events', () => {
         const div = new EventDom()
         expect(() => div.off()).not.toThrowError()
       })
+
+      it('should do nothing for unexist event', () => {
+        const div = new EventDom()
+        div.on('whatever', () => {})
+        expect(() => div.off('unexist')).not.toThrowError()
+      })
     })
 
     describe('trigger()', () => {

diff --git a/packages/x6-common/src/common/basecoat.ts b/packages/x6-common/src/common/basecoat.ts
@@ -3,7 +3,15 @@ import { EventArgs } from '../event/types'
 import { ObjectExt } from '../object'
 import { Disposable } from './disposable'
 
-export class Basecoat<A extends EventArgs = any> extends Events<A> {}
+export class Basecoat<A extends EventArgs = any>
+  extends Events<A>
+  implements Disposable
+{
+  @Disposable.dispose()
+  dispose() {
+    this.off()
+  }
+}
 
 export interface Basecoat extends Disposable {}
 

diff --git a/packages/x6-common/src/dom/event/core.ts b/packages/x6-common/src/dom/event/core.ts
@@ -148,7 +148,10 @@ export namespace Core {
       let type = originType
       const hook = EventHook.get(type)
       type = (selector ? hook.delegateType : hook.bindType) || type
-      const bag = events[type] || {}
+      const bag = events[type]
+      if (!bag) {
+        return
+      }
       const rns =
         namespaces.length > 0
           ? new RegExp(`(^|\\.)${namespaces.join('\\.(?:.*\\.|)')}(\\.|$)`)

diff --git a/packages/x6/src/graph/graph.ts b/packages/x6/src/graph/graph.ts
@@ -1262,6 +1262,7 @@ export class Graph extends Basecoat<EventArgs> {
     const aboutToChangePlugins = this.getPlugins(postPlugins)
     aboutToChangePlugins?.forEach((plugin) => {
       plugin.dispose()
+      this.installedPlugins.delete(plugin)
     })
     return this
   }
@@ -1273,11 +1274,9 @@ export class Graph extends Basecoat<EventArgs> {
   @Basecoat.dispose()
   dispose(clean = true) {
     if (clean) {
-      this.clearCells()
+      this.model.dispose()
     }
 
-    this.off()
-
     this.css.dispose()
     this.defs.dispose()
     this.grid.dispose()

diff --git a/packages/x6/src/model/collection.ts b/packages/x6/src/model/collection.ts
@@ -313,6 +313,11 @@ export class Collection extends Basecoat<Collection.EventArgs> {
     this.cells = []
     this.map = {}
   }
+
+  @Collection.dispose()
+  dispose() {
+    this.reset([])
+  }
 }
 
 export namespace Collection {

diff --git a/packages/x6/src/model/model.ts b/packages/x6/src/model/model.ts
@@ -1323,6 +1323,11 @@ export class Model extends Basecoat<Model.EventArgs> {
   }
 
   // #endregion
+
+  @Model.dispose()
+  dispose() {
+    this.collection.dispose()
+  }
 }
 
 export namespace Model {

diff --git a/packages/x6/src/renderer/scheduler.ts b/packages/x6/src/renderer/scheduler.ts
@@ -524,6 +524,11 @@ export class Scheduler extends Disposable {
   @Disposable.dispose()
   dispose() {
     this.stopListening()
+    // clear views
+    Object.keys(this.views).forEach((id) => {
+      this.views[id].view.dispose()
+    })
+    this.views = {}
   }
 }
 export namespace Scheduler {

diff --git a/packages/x6/src/shape/html.ts b/packages/x6/src/shape/html.ts
@@ -17,15 +17,17 @@ export namespace HTML {
   export class View extends NodeView<HTML> {
     protected init() {
       super.init()
-      this.cell.on('change:*', ({ key }) => {
-        const content = shapeMaps[this.cell.shape]
-        if (content) {
-          const { effect } = content
-          if (!effect || effect.includes(key)) {
-            this.renderHTMLComponent()
-          }
+      this.cell.on('change:*', this.onCellChangeAny, this)
+    }
+
+    protected onCellChangeAny({ key }: Cell.EventArgs['change:*']) {
+      const content = shapeMaps[this.cell.shape]
+      if (content) {
+        const { effect } = content
+        if (!effect || effect.includes(key)) {
+          this.renderHTMLComponent()
         }
-      })
+      }
     }
 
     confirmUpdate(flag: number) {
@@ -58,6 +60,11 @@ export namespace HTML {
         }
       }
     }
+
+    @View.dispose()
+    dispose() {
+      this.cell.off('change:*', this.onCellChangeAny, this)
+    }
   }
 
   export namespace View {

diff --git a/packages/x6/src/view/cell.ts b/packages/x6/src/view/cell.ts
@@ -251,7 +251,11 @@ export class CellView<
   }
 
   protected setup() {
-    this.cell.on('changed', ({ options }) => this.onAttrsChange(options))
+    this.cell.on('changed', this.onCellChanged, this)
+  }
+
+  protected onCellChanged({ options }: Cell.EventArgs['changed']) {
+    this.onAttrsChange(options)
   }
 
   protected onAttrsChange(options: Cell.MutateOptions) {
@@ -739,6 +743,11 @@ export class CellView<
     view.onMouseEnter(e as Dom.MouseEnterEvent)
   }
 
+  @CellView.dispose()
+  dispose() {
+    this.cell.off('changed', this.onCellChanged, this)
+  }
+
   // #endregion
 }
 

diff --git a/packages/x6/src/view/view.ts b/packages/x6/src/view/view.ts
@@ -362,6 +362,11 @@ export abstract class View<A extends EventArgs = any> extends Basecoat<A> {
   normalizeEvent<T extends Dom.EventObject>(evt: T) {
     return View.normalizeEvent(evt)
   }
+
+  @View.dispose()
+  dispose() {
+    this.remove()
+  }
 }
 
 export namespace View {