Repository: buildo/react-components (ts)

Issue:
[Button] no check before invoke setState() for still mounted component
## description
Sometimes buttons, e.g.: Save for comment in sample details throw a warning when clicked.
Check against still mounted component with isMounted() is an antipattern, so a proper solution is to make the promise cancelable, although the warning seems caused by doResetInternalState() call.

## how to reproduce
- Have a button which `onClick` cause a re-render

## specs
- check component is still **mounted** when `setState` is called after the `onClick` promise resolution.

## misc
[optional: other useful info]

Pull request description:
#280: [Button] no check before invoke setState() for still mounted component (closes #280)
Issue #280

After discussion in #283 I ended up with @giogonzo solution.
The tests are almost useless now, so maybe we need to rethink about them (in a separate pr).

I also defined global `it` and `describe` in the `.eslintrc` for tests lint.

Patch:
diff --git a/.eslintrc b/.eslintrc
index 551b5d89..cce4934c 100644
--- a/.eslintrc
+++ b/.eslintrc
@@ -1,3 +1,7 @@
 {
-  "extends": "buildo"
+  "extends": "buildo",
+  "globals": {
+    "describe": true,
+    "it": true,
+  }
 }
diff --git a/src/button/ButtonLogic.js b/src/button/ButtonLogic.js
index 65cb2a0e..fdd21ad3 100644
--- a/src/button/ButtonLogic.js
+++ b/src/button/ButtonLogic.js
@@ -63,10 +63,20 @@ export default class ButtonLogic extends React.Component {
     };
   }
 
-  doResetInternalState() {
-    this.setState({
-      internalState: null
-    });
+  componentDidMount() {
+    this._isMounted = true;
+  }
+
+  componentWillUnmount() {
+    this._isMounted = false;
+  }
+
+  doResetInternalState = () => {
+    if (this._isMounted) {
+      this.setState({
+        internalState: null
+      });
+    }
   }
 
   componentWillReceiveProps(props) {
@@ -89,28 +99,32 @@ export default class ButtonLogic extends React.Component {
     }
   }
 
-  doResetInternalStateAfterTimer() {
+  doResetInternalStateAfterTimer = () => {
     this.timeoutId = setTimeout(() => {
       this.timeoutId = null;
       this.doResetInternalState();
     }, this.props.timerMillis);
   }
 
-  attachPromiseHandlers(promise) {
+  attachPromiseHandlers = (promise) => {
     promise.then(() => {
-      this.setState({
-        internalState: 'success'
-      }, () => {
-        if (!this.props.stableSuccess) {
-          this.doResetInternalStateAfterTimer();
-        } else if (this.resetInternalStateAfterProcessing) {
-          this.doResetInternalState();
-        }
-      });
+      if (this._isMounted) {
+        this.setState({
+          internalState: 'success'
+        }, () => {
+          if (!this.props.stableSuccess) {
+            this.doResetInternalStateAfterTimer();
+          } else if (this.resetInternalStateAfterProcessing) {
+            this.doResetInternalState();
+          }
+        });
+      }
     }).catch(() => {
-      this.setState({
-        internalState: 'error'
-      }, () => this.doResetInternalStateAfterTimer() );
+      if (this._isMounted) {
+        this.setState({
+          internalState: 'error'
+        }, () => this.doResetInternalStateAfterTimer() );
+      }
     });
   }
 


Test patch:
diff --git a/test/tests/components/ButtonLogic.js b/test/tests/components/ButtonLogic.js
index 6026d938..6d954302 100644
--- a/test/tests/components/ButtonLogic.js
+++ b/test/tests/components/ButtonLogic.js
@@ -1,5 +1,4 @@
 import React from 'react/addons';
-const TestUtils = React.addons.TestUtils;
 import expect from 'expect';
 import ButtonLogic from '../../../src/button/ButtonLogic';
 import vdom from 'react-vdom';
@@ -8,7 +7,7 @@ import clone from 'lodash/lang/clone';
 describe('ButtonLogic', () => {
 
   function timeoutPromise(millis) {
-    return new Promise((resolve, reject) => {
+    return new Promise((resolve) => {
       setTimeout(resolve, millis);
     });
   }
@@ -24,7 +23,7 @@ describe('ButtonLogic', () => {
     });
     component.setState = (state, cb) => {
       component.state = state;
-      cb && cb()
+      cb && cb();
     };
     return component;
   }
@@ -58,6 +57,7 @@ describe('ButtonLogic', () => {
       const component = mkComponent({
         stableSuccess: false
       });
+      component.componentDidMount();
       component.getLocals().onClick();
       expect(component.getLocals().buttonState).toBe('processing');
       expect(vdom(component).attrs.buttonState).toBe('processing');
@@ -76,6 +76,7 @@ describe('ButtonLogic', () => {
       const component = mkComponent({
         stableSuccess: true
       });
+      component.componentDidMount();
       component.getLocals().onClick();
       expect(component.getLocals().buttonState).toBe('processing');
       return timeoutPromise(6).then(() => {
@@ -91,6 +92,7 @@ describe('ButtonLogic', () => {
       const component = mkComponent({
         stableSuccess: true
       });
+      component.componentDidMount();
       component.getLocals().onClick();
       expect(component.getLocals().buttonState).toBe('processing');
       const newProps = clone(component.props);