From c67e7569d6b0f50da1219430d10f6ededd6ce953 Mon Sep 17 00:00:00 2001 From: syn-zeta Date: Fri, 9 Feb 2018 00:35:29 -0800 Subject: [PATCH] Popper: Fix memory leak on route change --- src/utils/popper.js | 27 +++++++++++---------------- src/utils/vue-popper.js | 6 +++--- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/utils/popper.js b/src/utils/popper.js index 11b9418231..cb3b6d9d08 100644 --- a/src/utils/popper.js +++ b/src/utils/popper.js @@ -240,7 +240,6 @@ if (typeof this.state.updateCallback === 'function') { this.state.updateCallback(data); } - }; /** @@ -438,7 +437,6 @@ popperOffsets.width = popperRect.width; popperOffsets.height = popperRect.height; - return { popper: popperOffsets, reference: referenceOffsets @@ -464,6 +462,7 @@ target = root; } target.addEventListener('scroll', this.state.updateBound); + this.state.scrollTarget = target; } }; @@ -476,13 +475,9 @@ Popper.prototype._removeEventListeners = function() { // NOTE: 1 DOM access here root.removeEventListener('resize', this.state.updateBound); - if (this._options.boundariesElement !== 'window') { - var target = getScrollParent(this._reference); - // here it could be both `body` or `documentElement` thanks to Firefox, we then check both - if (target === root.document.body || target === root.document.documentElement) { - target = root; - } - target.removeEventListener('scroll', this.state.updateBound); + if (this._options.boundariesElement !== 'window' && this.state.scrollTarget) { + this.state.scrollTarget.removeEventListener('scroll', this.state.updateBound); + this.state.scrollTarget = null; } this.state.updateBound = null; }; @@ -519,13 +514,13 @@ var scrollParent = getScrollParent(this._popper); var offsetParentRect = getOffsetRect(offsetParent); - // Thanks the fucking native API, `document.body.scrollTop` & `document.documentElement.scrollTop` - var getScrollTopValue = function (element) { - return element == document.body ? Math.max(document.documentElement.scrollTop, document.body.scrollTop) : element.scrollTop; - } - var getScrollLeftValue = function (element) { - return element == document.body ? Math.max(document.documentElement.scrollLeft, document.body.scrollLeft) : element.scrollLeft; - } + // Thanks the fucking native API, `document.body.scrollTop` & `document.documentElement.scrollTop` + var getScrollTopValue = function (element) { + return element == document.body ? Math.max(document.documentElement.scrollTop, document.body.scrollTop) : element.scrollTop; + } + var getScrollLeftValue = function (element) { + return element == document.body ? Math.max(document.documentElement.scrollLeft, document.body.scrollLeft) : element.scrollLeft; + } // if the popper is fixed we don't have to substract scrolling from the boundaries var scrollTop = data.offsets.popper.position === 'fixed' ? 0 : getScrollTopValue(scrollParent); diff --git a/src/utils/vue-popper.js b/src/utils/vue-popper.js index 57b693c410..377731108f 100644 --- a/src/utils/vue-popper.js +++ b/src/utils/vue-popper.js @@ -129,9 +129,9 @@ export default { } }, - doDestroy() { + doDestroy(forceDestroy) { /* istanbul ignore if */ - if (this.showPopper || !this.popperJS) return; + if (!this.popperJS || (this.showPopper && !forceDestroy)) return; this.popperJS.destroy(); this.popperJS = null; }, @@ -184,7 +184,7 @@ export default { }, beforeDestroy() { - this.doDestroy(); + this.doDestroy(true); if (this.popperElm && this.popperElm.parentNode === document.body) { this.popperElm.removeEventListener('click', stop); document.body.removeChild(this.popperElm);