From ffbc7edbb55fb4041bdaeb8ab8a3e8475a214bcb Mon Sep 17 00:00:00 2001 From: cyrilluce Date: Mon, 6 Nov 2023 19:42:26 +0800 Subject: [PATCH] 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 --- .../x6-common/src/__tests__/dom/event.test.ts | 8 ++++++- packages/x6-common/src/common/basecoat.ts | 10 +++++++- packages/x6-common/src/dom/event/core.ts | 5 +++- packages/x6/src/graph/graph.ts | 5 ++-- packages/x6/src/model/collection.ts | 5 ++++ packages/x6/src/model/model.ts | 5 ++++ packages/x6/src/renderer/scheduler.ts | 5 ++++ packages/x6/src/shape/html.ts | 23 ++++++++++++------- packages/x6/src/view/cell.ts | 11 ++++++++- packages/x6/src/view/view.ts | 5 ++++ 10 files changed, 67 insertions(+), 15 deletions(-) diff --git a/packages/x6-common/src/__tests__/dom/event.test.ts b/packages/x6-common/src/__tests__/dom/event.test.ts index ad73f0026ee..e64520194bb 100644 --- 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 index 92a4b2178f5..7cefddba65a 100644 --- 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 extends Events {} +export class Basecoat + extends Events + 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 index 19552882be8..e37a9171957 100644 --- 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 index 8e2dc5bb390..ea57fdf1980 100644 --- a/packages/x6/src/graph/graph.ts +++ b/packages/x6/src/graph/graph.ts @@ -1262,6 +1262,7 @@ export class Graph extends Basecoat { 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 { @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 index 83eb3b43c4c..38c38918cd1 100644 --- a/packages/x6/src/model/collection.ts +++ b/packages/x6/src/model/collection.ts @@ -313,6 +313,11 @@ export class Collection extends Basecoat { 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 index f30b71e765e..a40912ccd55 100644 --- a/packages/x6/src/model/model.ts +++ b/packages/x6/src/model/model.ts @@ -1323,6 +1323,11 @@ export class Model extends Basecoat { } // #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 index 47f060e94dc..7abddb92696 100644 --- 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 index 4a2e5c19412..83330737749 100644 --- 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 { 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 index 65a6406c993..970c1135f7c 100644 --- 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 index 806037a8b86..d07366d28ef 100644 --- a/packages/x6/src/view/view.ts +++ b/packages/x6/src/view/view.ts @@ -362,6 +362,11 @@ export abstract class View extends Basecoat { normalizeEvent(evt: T) { return View.normalizeEvent(evt) } + + @View.dispose() + dispose() { + this.remove() + } } export namespace View {