From a1873d0d9ce6d658bde27218875795b8cd7ad5db Mon Sep 17 00:00:00 2001 From: Yannis JUSTINE <77580217+YannisJustine@users.noreply.github.com> Date: Mon, 13 Jan 2025 11:58:23 +0100 Subject: [PATCH] Fix memory leaks (#6889) * Fix most of memory leaks - Remove callback after component is unmounted * fix: restore original unmounted implementation in BaseComponent * Remove callback map --- .../core/src/basecomponent/BaseComponent.vue | 38 +++++++++++-------- .../core/src/basedirective/BaseDirective.js | 29 ++++++++++++-- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/packages/core/src/basecomponent/BaseComponent.vue b/packages/core/src/basecomponent/BaseComponent.vue index 937dac468..1f308debe 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,6 +105,8 @@ export default { this._hook('onBeforeUnmount'); }, unmounted() { + ThemeService.off('theme:change', this._loadCoreStyles); + ThemeService.off('theme:change', this._load); this._unloadScopedThemeStyles(); this._hook('onUnmounted'); }, @@ -116,21 +123,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..501016ab9 100644 --- a/packages/core/src/basedirective/BaseDirective.js +++ b/packages/core/src/basedirective/BaseDirective.js @@ -76,7 +76,11 @@ const BaseDirective = { BaseDirective._loadThemeStyles(el.$instance, useStyleOptions); BaseDirective._loadScopedThemeStyles(el.$instance, useStyleOptions); - BaseDirective._themeChangeListener(() => BaseDirective._loadThemeStyles(el.$instance, useStyleOptions)); + BaseDirective._removeThemeListeners(el.$instance); + + el.$instance.$loadStyles = () => BaseDirective._loadThemeStyles(el.$instance, useStyleOptions); + + BaseDirective._themeChangeListener(el.$instance.$loadStyles); }, _loadCoreStyles(instance = {}, useStyleOptions) { if (!Base.isStyleNameLoaded(instance.$style?.name) && instance.$style?.name) { @@ -134,6 +138,9 @@ const BaseDirective = { Base.clearLoadedStyleNames(); ThemeService.on('theme:change', callback); }, + _removeThemeListeners(instance) { + ThemeService.off('theme:change', instance.$loadStyles); + }, _hook: (directiveName, hookName, el, binding, vnode, prevVnode) => { const name = `on${toCapitalCase(hookName)}`; const config = BaseDirective._getConfig(binding, vnode); @@ -193,13 +200,27 @@ 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); + + el.$instance.$watchersCallback = { 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 = el.$instance.$watchersCallback; + + if (watchers) { + PrimeVueService.off('config:change', watchers.config); + PrimeVueService.off('config:ripple:change', watchers['config.ripple']); + } }; return { @@ -225,6 +246,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) => {