Ensures listeners get uninstalled when dom node changes or component …

…unmounts
ctrlplusb · May 1, 2019 · b11218b · b11218b
1 parent 261c349
commit b11218b
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 11 deletions.
diff --git a/src/__tests__/with-size.test.js b/src/__tests__/with-size.test.js
@@ -215,7 +215,7 @@ describe('withSize', () => {
 
       // An add listener should have been called for the placeholder.
       expect(resizeDetectorMock.listenTo).toHaveBeenCalledTimes(1)
-      expect(resizeDetectorMock.removeAllListeners).toHaveBeenCalledTimes(0)
+      expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(0)
 
       // Get the callback for size changes.
       const checkIfSizeChangedCallback =
@@ -232,14 +232,14 @@ describe('withSize', () => {
       // on the newly mounted component.
       expect(mounted.text()).toEqual(expected({ width: 100, height: 50 }))
       expect(resizeDetectorMock.listenTo).toHaveBeenCalledTimes(2)
-      expect(resizeDetectorMock.removeAllListeners).toHaveBeenCalledTimes(1)
+      expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(1)
 
       // umount
       mounted.unmount()
 
       // The remove listener should have been called!
       expect(resizeDetectorMock.listenTo).toHaveBeenCalledTimes(2)
-      expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(1)
+      expect(resizeDetectorMock.uninstall).toHaveBeenCalledTimes(2)
     })
   })
 

diff --git a/src/with-size.js b/src/with-size.js
@@ -10,6 +10,9 @@ import throttle from 'lodash.throttle'
 import debounce from 'lodash.debounce'
 import resizeDetector from './resize-detector'
 
+const errMsg =
+  'react-sizeme: an error occurred whilst stopping to listen to node size changes'
+
 const defaultConfig = {
   monitorWidth: true,
   monitorHeight: false,
@@ -203,9 +206,17 @@ function withSize(config = defaultConfig) {
         // late running events.
         this.hasSizeChanged = () => undefined
         this.checkIfSizeChanged = () => undefined
+        this.uninstall()
+      }
 
+      uninstall = () => {
         if (this.domEl) {
-          this.detector.uninstall(this.domEl)
+          try {
+            this.detector.uninstall(this.domEl)
+          } catch (err) {
+            // eslint-disable-next-line no-console
+            console.warn(errMsg)
+          }
           this.domEl = null
         }
       }
@@ -240,18 +251,15 @@ function withSize(config = defaultConfig) {
         if (!found) {
           // If we previously had a dom node then we need to ensure that
           // we remove any existing listeners to avoid memory leaks.
-          if (this.domEl) {
-            this.detector.removeAllListeners(this.domEl)
-            this.domEl = null
-          }
+          this.uninstall()
           return
         }
 
         if (!this.domEl) {
           this.domEl = found
           this.detector.listenTo(this.domEl, this.checkIfSizeChanged)
         } else if (!this.domEl.isSameNode(found)) {
-          this.detector.removeAllListeners(this.domEl)
+          this.uninstall()
           this.domEl = found
           this.detector.listenTo(this.domEl, this.checkIfSizeChanged)
         } else {
@@ -270,13 +278,13 @@ function withSize(config = defaultConfig) {
         const np = n.position || {}
 
         return (
+          (monitorWidth && c.width !== n.width) ||
           (monitorHeight && c.height !== n.height) ||
           (monitorPosition &&
             (cp.top !== np.top ||
               cp.left !== np.left ||
               cp.bottom !== np.bottom ||
-              cp.right !== np.right)) ||
-          (monitorWidth && c.width !== n.width)
+              cp.right !== np.right))
         )
       }