Skip to content

Commit b11218b

Browse files
committed
Ensures listeners get uninstalled when dom node changes or component unmounts
1 parent 261c349 commit b11218b

2 files changed

Lines changed: 19 additions & 11 deletions

File tree

src/__tests__/with-size.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ describe('withSize', () => {
215215

216216
// An add listener should have been called for the placeholder.
217217
expect(resizeDetectorMock.listenTo).toHaveBeenCalledTimes(1)
218-
expect(resizeDetectorMock.removeAllListeners).toHaveBeenCalledTimes(0)
218+
expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(0)
219219

220220
// Get the callback for size changes.
221221
const checkIfSizeChangedCallback =
@@ -232,14 +232,14 @@ describe('withSize', () => {
232232
// on the newly mounted component.
233233
expect(mounted.text()).toEqual(expected({ width: 100, height: 50 }))
234234
expect(resizeDetectorMock.listenTo).toHaveBeenCalledTimes(2)
235-
expect(resizeDetectorMock.removeAllListeners).toHaveBeenCalledTimes(1)
235+
expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(1)
236236

237237
// umount
238238
mounted.unmount()
239239

240240
// The remove listener should have been called!
241241
expect(resizeDetectorMock.listenTo).toHaveBeenCalledTimes(2)
242-
expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(1)
242+
expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(2)
243243
})
244244
})
245245

src/with-size.js

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import throttle from 'lodash.throttle'
1010
import debounce from 'lodash.debounce'
1111
import resizeDetector from './resize-detector'
1212

13+
const errMsg =
14+
'react-sizeme: an error occurred whilst stopping to listen to node size changes'
15+
1316
const defaultConfig = {
1417
monitorWidth: true,
1518
monitorHeight: false,
@@ -203,9 +206,17 @@ function withSize(config = defaultConfig) {
203206
// late running events.
204207
this.hasSizeChanged = () => undefined
205208
this.checkIfSizeChanged = () => undefined
209+
this.uninstall()
210+
}
206211

212+
uninstall = () => {
207213
if (this.domEl) {
208-
this.detector.uninstall(this.domEl)
214+
try {
215+
this.detector.uninstall(this.domEl)
216+
} catch (err) {
217+
// eslint-disable-next-line no-console
218+
console.warn(errMsg)
219+
}
209220
this.domEl = null
210221
}
211222
}
@@ -240,18 +251,15 @@ function withSize(config = defaultConfig) {
240251
if (!found) {
241252
// If we previously had a dom node then we need to ensure that
242253
// we remove any existing listeners to avoid memory leaks.
243-
if (this.domEl) {
244-
this.detector.removeAllListeners(this.domEl)
245-
this.domEl = null
246-
}
254+
this.uninstall()
247255
return
248256
}
249257

250258
if (!this.domEl) {
251259
this.domEl = found
252260
this.detector.listenTo(this.domEl, this.checkIfSizeChanged)
253261
} else if (!this.domEl.isSameNode(found)) {
254-
this.detector.removeAllListeners(this.domEl)
262+
this.uninstall()
255263
this.domEl = found
256264
this.detector.listenTo(this.domEl, this.checkIfSizeChanged)
257265
} else {
@@ -270,13 +278,13 @@ function withSize(config = defaultConfig) {
270278
const np = n.position || {}
271279

272280
return (
281+
(monitorWidth && c.width !== n.width) ||
273282
(monitorHeight && c.height !== n.height) ||
274283
(monitorPosition &&
275284
(cp.top !== np.top ||
276285
cp.left !== np.left ||
277286
cp.bottom !== np.bottom ||
278-
cp.right !== np.right)) ||
279-
(monitorWidth && c.width !== n.width)
287+
cp.right !== np.right))
280288
)
281289
}
282290

0 commit comments

Comments
 (0)