From e165c82513101d7d6d4d198545fbb79556d6011e Mon Sep 17 00:00:00 2001 From: YannisJustine Date: Thu, 28 Nov 2024 21:50:26 +0100 Subject: [PATCH] Fix most of memory leaks - Remove callback after component is unmounted --- .../core/src/basecomponent/BaseComponent.vue | 39 ++++++++------ .../core/src/basedirective/BaseDirective.js | 54 +++++++++++++++++-- 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/packages/core/src/basecomponent/BaseComponent.vue b/packages/core/src/basecomponent/BaseComponent.vue index 937dac468..cd9561f9a 100644 --- a/packages/core/src/basecomponent/BaseComponent.vue +++ b/packages/core/src/basecomponent/BaseComponent.vue @@ -45,10 +45,15 @@ export default { }, dt: { immediate: true, - handler(newValue) { + handler(newValue, oldValue) { + if (oldValue) { + ThemeService.off('theme:change', this._themeScopedListener); + } + if (newValue) { this._loadScopedThemeStyles(newValue); - this._themeChangeListener(() => this._loadScopedThemeStyles(newValue)); + this._themeScopedListener = () => this._loadScopedThemeStyles(newValue); + this._themeChangeListener(this._themeScopedListener) } else { this._unloadScopedThemeStyles(); } @@ -100,7 +105,8 @@ export default { this._hook('onBeforeUnmount'); }, unmounted() { - this._unloadScopedThemeStyles(); + ThemeService.off('theme:change', this._loadCoreStyles); + ThemeService.off('theme:change', this._load); this._hook('onUnmounted'); }, methods: { @@ -116,21 +122,20 @@ export default { _mergeProps(fn, ...args) { return isFunction(fn) ? fn(...args) : mergeProps(...args); }, + _load() { + // @todo + if (!Base.isStyleNameLoaded('base')) { + BaseStyle.loadCSS(this.$styleOptions); + this._loadGlobalStyles(); + + Base.setLoadedStyleName('base'); + } + + this._loadThemeStyles(); + }, _loadStyles() { - const _load = () => { - // @todo - if (!Base.isStyleNameLoaded('base')) { - BaseStyle.loadCSS(this.$styleOptions); - this._loadGlobalStyles(); - - Base.setLoadedStyleName('base'); - } - - this._loadThemeStyles(); - }; - - _load(); - this._themeChangeListener(_load); + this._load(); + this._themeChangeListener(this._load); }, _loadCoreStyles() { if (!Base.isStyleNameLoaded(this.$style?.name) && this.$style?.name) { diff --git a/packages/core/src/basedirective/BaseDirective.js b/packages/core/src/basedirective/BaseDirective.js index 28b9eb4f3..95aa1283b 100644 --- a/packages/core/src/basedirective/BaseDirective.js +++ b/packages/core/src/basedirective/BaseDirective.js @@ -6,6 +6,9 @@ import BaseStyle from '@primevue/core/base/style'; import PrimeVueService from '@primevue/core/service'; import { mergeProps } from 'vue'; +const _themesCallback = new Map(); +const _configCallback = new Map(); + const BaseDirective = { _getMeta: (...args) => [isObject(args[0]) ? undefined : args[0], resolve(isObject(args[0]) ? args[0] : args[1])], _getConfig: (binding, vnode) => (binding?.instance?.$primevue || vnode?.ctx?.appContext?.config?.globalProperties?.$primevue)?.config, @@ -76,7 +79,9 @@ const BaseDirective = { BaseDirective._loadThemeStyles(el.$instance, useStyleOptions); BaseDirective._loadScopedThemeStyles(el.$instance, useStyleOptions); - BaseDirective._themeChangeListener(() => BaseDirective._loadThemeStyles(el.$instance, useStyleOptions)); + const loadStyle = () => BaseDirective._loadThemeStyles(el.$instance, useStyleOptions) + + BaseDirective._themeChangeListener(el.$instance, loadStyle); }, _loadCoreStyles(instance = {}, useStyleOptions) { if (!Base.isStyleNameLoaded(instance.$style?.name) && instance.$style?.name) { @@ -130,10 +135,33 @@ const BaseDirective = { instance.scopedStyleEl = scopedStyle.el; } }, - _themeChangeListener(callback = () => {}) { + _themeChangeListener(instance, callback = () => {}) { + if (!instance || !callback) return; + + // Store callback reference + let listeners = _themesCallback.get(instance.$attrSelector); + + if (!listeners) { + listeners = new Set(); + _themesCallback.set(instance.$attrSelector, listeners); + } + + listeners.add(callback); + Base.clearLoadedStyleNames(); ThemeService.on('theme:change', callback); }, + _removeThemeListeners(instance) { + const listeners = _themesCallback.get(instance.$attrSelector); + + if (listeners) { + listeners.forEach(callback => { + ThemeService.off('theme:change', callback); + }); + listeners.clear(); + _themesCallback.delete(instance); + } + }, _hook: (directiveName, hookName, el, binding, vnode, prevVnode) => { const name = `on${toCapitalCase(hookName)}`; const config = BaseDirective._getConfig(binding, vnode); @@ -193,15 +221,31 @@ const BaseDirective = { const handleWatch = (el) => { const watchers = el.$instance?.watch; + + const handleWatchConfig = ({ newValue, oldValue }) => watchers?.['config']?.call(el.$instance, newValue, oldValue); + const handleWatchConfigRipple = ({ newValue, oldValue }) => watchers?.['config.ripple']?.call(el.$instance, newValue, oldValue) + + _configCallback.set(el, { config: handleWatchConfig, 'config.ripple': handleWatchConfigRipple }); + // for 'config' watchers?.['config']?.call(el.$instance, el.$instance?.$primevueConfig); - PrimeVueService.on('config:change', ({ newValue, oldValue }) => watchers?.['config']?.call(el.$instance, newValue, oldValue)); + PrimeVueService.on('config:change', handleWatchConfig); // for 'config.ripple' watchers?.['config.ripple']?.call(el.$instance, el.$instance?.$primevueConfig?.ripple); - PrimeVueService.on('config:ripple:change', ({ newValue, oldValue }) => watchers?.['config.ripple']?.call(el.$instance, newValue, oldValue)); + PrimeVueService.on('config:ripple:change', handleWatchConfigRipple); }; + const removeWatch = (el) => { + const watchers = _configCallback.get(el); + + if (watchers) { + PrimeVueService.off('config:change', watchers.config); + PrimeVueService.off('config:ripple:change', watchers['config.ripple']); + _configCallback.delete(el); + } + } + return { created: (el, binding, vnode, prevVnode) => { el.$pd ||= {}; @@ -225,6 +269,8 @@ const BaseDirective = { handleHook('updated', el, binding, vnode, prevVnode); }, beforeUnmount: (el, binding, vnode, prevVnode) => { + removeWatch(el); + BaseDirective._removeThemeListeners(el.$instance); handleHook('beforeUnmount', el, binding, vnode, prevVnode); }, unmounted: (el, binding, vnode, prevVnode) => {