Skip to content

Commit 05a5c50

Browse files
authored
fix(dropdowns): prevent memory leak on destroy (bootstrap-vue#1392)
* fix(dropdowns): remove memory leak on destroy Also, we don't bother instantiating Popper if dropdown is in a navbar. Closes issue bootstrap-vue#1391 * ESLint
1 parent 8104cb4 commit 05a5c50

File tree

1 file changed

+31
-31
lines changed

1 file changed

+31
-31
lines changed

src/mixins/dropdown.js

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import clickoutMixin from './clickout'
33
import listenOnRootMixin from './listen-on-root'
44
import { from as arrayFrom } from '../utils/array'
55
import { assign } from '../utils/object'
6-
import { KeyCodes } from '../utils'
6+
import { KeyCodes, warn } from '../utils'
77
import { isVisible, closest, selectAll, getAttr, eventOn, eventOff } from '../utils/dom'
88

99
// Return an Array of visible items
@@ -71,7 +71,7 @@ export default {
7171
inNavbar: null
7272
}
7373
},
74-
created () {
74+
mounted () {
7575
const listener = vm => {
7676
if (vm !== this) {
7777
this.visible = false
@@ -86,6 +86,14 @@ export default {
8686
// Use new namespaced events
8787
this.listenOnRoot('bv::link::clicked', listener)
8888
},
89+
beforeDestroy () {
90+
if (this._popper) {
91+
// Ensure popper event listeners are removed cleanly
92+
this._popper.destroy()
93+
}
94+
this._popper = null
95+
this.setTouchStart(false)
96+
},
8997
watch: {
9098
visible (state, old) {
9199
if (state === old) {
@@ -110,14 +118,6 @@ export default {
110118
return this.$refs.toggle.$el || this.$refs.toggle
111119
}
112120
},
113-
destroyed () {
114-
if (this._popper) {
115-
// Ensure popper event listeners are removed cleanly
116-
this._popper.destroy()
117-
}
118-
this._popper = null
119-
this.setTouchStart(false)
120-
},
121121
methods: {
122122
showMenu () {
123123
if (this.disabled) {
@@ -128,19 +128,23 @@ export default {
128128
// Ensure other menus are closed
129129
this.emitOnRoot('bv::dropdown::shown', this)
130130

131-
// If popper not installed, then fallback gracefully to dropdown only with left alignment
132-
if (typeof Popper === 'function') {
133-
// Are we in a navbar ?
134-
if (this.inNavbar === null && this.isNav) {
135-
this.inNavbar = Boolean(closest('.navbar', this.$el))
136-
}
137-
// for dropup with alignment we use the parent element as popper container
138-
let element = ((this.dropup && this.right) || this.split || this.inNavbar) ? this.$el : this.$refs.toggle
139-
// Make sure we have a reference to an element, not a component!
140-
element = element.$el || element
131+
// Are we in a navbar ?
132+
if (this.inNavbar === null && this.isNav) {
133+
this.inNavbar = Boolean(closest('.navbar', this.$el))
134+
}
141135

142-
// Instantiate popper.js
143-
this._popper = new Popper(element, this.$refs.menu, this.getPopperConfig())
136+
// Disable totally Popper.js for Dropdown in Navbar
137+
if (!this.inNavbar) {
138+
if (typeof Popper === 'undefined') {
139+
warn('b-dropdown: Popper.js not found. Falling back to CSS positioning.')
140+
} else {
141+
// for dropup with alignment we use the parent element as popper container
142+
let element = ((this.dropup && this.right) || this.split) ? this.$el : this.$refs.toggle
143+
// Make sure we have a reference to an element, not a component!
144+
element = element.$el || element
145+
// Instantiate popper.js
146+
this._popper = new Popper(element, this.$refs.menu, this.getPopperConfig())
147+
}
144148
}
145149

146150
this.setTouchStart(true)
@@ -181,22 +185,18 @@ export default {
181185
},
182186
flip: {
183187
enabled: !this.noFlip
184-
},
185-
applyStyle: {
186-
// Disable Popper.js for Dropdown in Navbar
187-
enabled: !this.inNavbar
188188
}
189189
}
190190
}
191191
return assign(popperConfig, this.popperOpts || {})
192192
},
193193
setTouchStart (on) {
194194
/*
195-
If this is a touch-enabled device we add extra
196-
empty mouseover listeners to the body's immediate children;
197-
only needed because of broken event delegation on iOS
198-
https://www.quirksmode.org/blog/archives/2014/02/mouse_event_bub.html
199-
*/
195+
* If this is a touch-enabled device we add extra
196+
* empty mouseover listeners to the body's immediate children;
197+
* only needed because of broken event delegation on iOS
198+
* https://www.quirksmode.org/blog/archives/2014/02/mouse_event_bub.html
199+
*/
200200
if ('ontouchstart' in document.documentElement) {
201201
const children = arrayFrom(document.body.children)
202202
children.forEach(el => {

0 commit comments

Comments
 (0)