Merge pull request tanem#188 from tanem/issue-187

Ensure setState isn't called when the component is unmounted

Author: Tane Morgan

src/index.tsx

@@ -72,6 +72,9 @@ export default class ReactSVG extends React.Component<
 
   state = this.initialState
 
+  // tslint:disable-next-line:variable-name
+  _isMounted = false
+
   container?: HTMLSpanElement | HTMLDivElement | null
 
   svgWrapper?: HTMLSpanElement | HTMLDivElement | null
@@ -108,15 +111,19 @@ export default class ReactSVG extends React.Component<
           this.removeSVG()
         }
 
-        this.setState(
-          () => ({
-            hasError: !!error,
-            isLoading: false
-          }),
-          () => {
-            onInjected(error, svg)
-          }
-        )
+        // TODO: It'd be better to cleanly unsubscribe from SVGInjector
+        // callbacks instead of tracking a property like this.
+        if (this._isMounted) {
+          this.setState(
+            () => ({
+              hasError: !!error,
+              isLoading: false
+            }),
+            () => {
+              onInjected(error, svg)
+            }
+          )
+        }
       }
 
       SVGInjector(this.svgWrapper.firstChild, {
@@ -135,6 +142,7 @@ export default class ReactSVG extends React.Component<
   }
 
   componentDidMount() {
+    this._isMounted = true
     this.renderSVG()
   }
 
@@ -151,6 +159,7 @@ export default class ReactSVG extends React.Component<
   }
 
   componentWillUnmount() {
+    this._isMounted = false
     this.removeSVG()
   }
 

test/browser.spec.tsx

@@ -246,4 +246,27 @@ describe('while running in a browser environment', () => {
 
     expect(wrapper.html()).toMatchSnapshot()
   })
+
+  // TODO: When we have the ability to cleanly unsubscribe from SVGInjector
+  // callbacks, we can update this dicey test to instead ensure the callbacks
+  // aren't called.
+  it('does not call setState when the component is unmounted', () => {
+    /* tslint:disable:no-console */
+    const orglFn = console.error
+    console.error = jest.fn()
+
+    wrapper = mount(
+      <ReactSVG src={`http://localhost/${faker.random.uuid()}.svg`} />
+    )
+
+    wrapper.unmount()
+
+    requests[0].respond(200, {}, source)
+    jest.runAllTimers()
+
+    expect(console.error).not.toHaveBeenCalled()
+
+    console.error = orglFn
+    /* tslint:enable:no-console */
+  })
 })