Merge pull request #2739 from alitaheri/fix-progress-timer-leak

Utilize cancellation over isMounted for indicators
mui · Dec 31, 2015 · 6b2b35c · 6b2b35c
2 parents 9ce1cd2 + 243c9ff
commit 6b2b35c
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 59 deletions.
diff --git a/docs/src/app/components/pages/components/Snackbar/ExampleTwice.jsx b/docs/src/app/components/pages/components/Snackbar/ExampleTwice.jsx
@@ -10,19 +10,19 @@ export default class SnackbarExampleTwice extends React.Component {
       message: 'Event added to your calendar',
       open: false,
     };
-    this._timerId = undefined;
+    this.timer = undefined;
   }
 
   componentWillUnMount() {
-    clearTimeout(this._timerId);
+    clearTimeout(this.timer);
   }
 
   handleTouchTap = () => {
     this.setState({
       open: true,
     });
 
-    this._timerId = setTimeout(() => {
+    this.timer = setTimeout(() => {
       this.setState({
         message: 'Event ' + Math.round(Math.random() * 100) + ' added to your calendar',
       });

diff --git a/docs/src/app/components/pages/components/progress.jsx b/docs/src/app/components/pages/components/progress.jsx
@@ -7,27 +7,30 @@ import CodeBlock from '../../CodeExample/CodeBlock';
 
 const ProgressPage = React.createClass({
 
+  timer: undefined,
+
   getInitialState() {
     return {
       completed: 0,
     };
   },
 
   componentDidMount() {
-    let self = this;
-
-    let id = window.setInterval(() => {
-
-      let diff = Math.random() * 10;
+    this.timer = setTimeout(() => this._progress(5), 1000);
+  },
 
-      self.setState({
-        completed: self.state.completed + diff,
-      });
+  _progress(completed) {
+    if (completed > 100) {
+      this.setState({completed: 100});
+    } else {
+      this.setState({completed});
+      const diff = Math.random() * 10;
+      this.timer = setTimeout(() => this._progress(completed + diff), 1000);
+    }
+  },
 
-      if (self.state.completed > 100) {
-        window.clearInterval(id);
-      }
-    }, 1000);
+  componentWillUnmount() {
+    clearTimeout(this.timer);
   },
 
   render() {

diff --git a/src/circular-progress.jsx b/src/circular-progress.jsx
@@ -10,6 +10,9 @@ const CircularProgress = React.createClass({
 
   mixins: [StylePropable],
 
+  scalePathTimer: undefined,
+  rotateWrapperTimer: undefined,
+
   propTypes: {
     color: React.PropTypes.string,
     innerStyle: React.PropTypes.object,
@@ -72,15 +75,17 @@ const CircularProgress = React.createClass({
     this._rotateWrapper(wrapper);
   },
 
+  componentWillUnmount() {
+    clearTimeout(this.scalePathTimer);
+    clearTimeout(this.rotateWrapperTimer);
+  },
+
   _scalePath(path, step) {
+    if (this.props.mode !== 'indeterminate') return;
+
     step = step || 0;
     step %= 3;
 
-    setTimeout(this._scalePath.bind(this, path, step + 1), step ? 750 : 250);
-
-    if (!this.isMounted()) return;
-    if (this.props.mode !== 'indeterminate') return;
-
     if (step === 0) {
       path.style.strokeDasharray = '1, 200';
       path.style.strokeDashoffset = 0;
@@ -96,12 +101,11 @@ const CircularProgress = React.createClass({
       path.style.strokeDashoffset = -124;
       path.style.transitionDuration = '850ms';
     }
+
+    this.scalePathTimer = setTimeout(() => this._scalePath(path, step + 1), step ? 750 : 250);
   },
 
   _rotateWrapper(wrapper) {
-    setTimeout(this._rotateWrapper.bind(this, wrapper), 10050);
-
-    if (!this.isMounted()) return;
     if (this.props.mode !== 'indeterminate') return;
 
     AutoPrefix.set(wrapper.style, 'transform', 'rotate(0deg)');
@@ -112,6 +116,8 @@ const CircularProgress = React.createClass({
       AutoPrefix.set(wrapper.style, 'transitionDuration', '10s');
       AutoPrefix.set(wrapper.style, 'transitionTimingFunction', 'linear');
     }, 50);
+
+    this.rotateWrapperTimer = setTimeout(() => this._rotateWrapper(wrapper), 10050);
   },
 
   getDefaultProps() {

diff --git a/src/linear-progress.jsx b/src/linear-progress.jsx
@@ -9,6 +9,11 @@ const LinearProgress = React.createClass({
 
   mixins: [StylePropable],
 
+  timers: {
+    bar1: undefined,
+    bar2: undefined,
+  },
+
   propTypes: {
     color: React.PropTypes.string,
     max: React.PropTypes.number,
@@ -65,25 +70,29 @@ const LinearProgress = React.createClass({
     let bar1 = ReactDOM.findDOMNode(this.refs.bar1);
     let bar2 = ReactDOM.findDOMNode(this.refs.bar2);
 
-    this._barUpdate(0, bar1, [
+    this.timers.bar1 = this._barUpdate('bar1', 0, bar1, [
       [-35, 100],
       [100, -90],
     ]);
 
-    setTimeout(() => {
-      this._barUpdate(0, bar2, [
+    this.timers.bar2 = setTimeout(() => {
+      this._barUpdate('bar2', 0, bar2, [
         [-200, 100],
         [107, -8],
       ]);
     }, 850);
   },
 
-  _barUpdate(step, barElement, stepValues) {
+  componentWillUnmount() {
+    clearTimeout(this.timers.bar1);
+    clearTimeout(this.timers.bar2);
+  },
+
+  _barUpdate(id, step, barElement, stepValues) {
+    if (this.props.mode !== 'indeterminate') return;
+
     step = step || 0;
     step %= 4;
-    setTimeout(this._barUpdate.bind(this, step + 1, barElement, stepValues), 420);
-    if (!this.isMounted()) return;
-    if (this.props.mode !== 'indeterminate') return;
 
     const right = this.state.muiTheme.isRtl ? 'left' : 'right';
     const left = this.state.muiTheme.isRtl ? 'right' : 'left';
@@ -102,6 +111,7 @@ const LinearProgress = React.createClass({
     else if (step === 3) {
       barElement.style.transitionDuration = '0ms';
     }
+    this.timers[id] = setTimeout(() => this._barUpdate(id, step + 1, barElement, stepValues), 420);
   },
 
   getDefaultProps() {

diff --git a/src/refresh-indicator.jsx b/src/refresh-indicator.jsx
@@ -11,6 +11,10 @@ const VIEWBOX_SIZE = 32;
 const RefreshIndicator = React.createClass({
   mixins: [StylePropable],
 
+  scalePathTimer: undefined,
+  rotateWrapperTimer: undefined,
+  rotateWrapperSecondTimer: undefined,
+
   contextTypes: {
     muiTheme: React.PropTypes.object,
   },
@@ -71,6 +75,12 @@ const RefreshIndicator = React.createClass({
     this._rotateWrapper(ReactDOM.findDOMNode(this.refs.wrapper));
   },
 
+  componentWillUnmount() {
+    clearTimeout(this.scalePathTimer);
+    clearTimeout(this.rotateWrapperTimer);
+    clearTimeout(this.rotateWrapperSecondTimer);
+  },
+
   render() {
     const rootStyle = this._getRootStyle();
     return (
@@ -250,13 +260,10 @@ const RefreshIndicator = React.createClass({
   },
 
   _scalePath(path, step) {
-    if (this.props.status !== 'loading' || !this.isMounted()) return;
+    if (this.props.status !== 'loading') return;
 
     const currStep = (step || 0) % 3;
 
-    clearTimeout(this._timer1);
-    this._timer1 = setTimeout(this._scalePath.bind(this, path, currStep + 1), currStep ? 750 : 250);
-
     const circle = this._getCircleAttr();
     const perimeter = Math.PI * 2 * circle.radiu;
     const arcLen = perimeter * 0.64;
@@ -279,25 +286,24 @@ const RefreshIndicator = React.createClass({
     AutoPrefix.set(path.style, 'strokeDasharray', strokeDasharray);
     AutoPrefix.set(path.style, 'strokeDashoffset', strokeDashoffset);
     AutoPrefix.set(path.style, 'transitionDuration', transitionDuration);
+
+    this.scalePathTimer = setTimeout(() => this._scalePath(path, currStep + 1), currStep ? 750 : 250);
   },
 
   _rotateWrapper(wrapper) {
-    if (this.props.status !== 'loading' || !this.isMounted()) return;
-
-    clearTimeout(this._timer2);
-    this._timer2 = setTimeout(this._rotateWrapper.bind(this, wrapper), 10050);
+    if (this.props.status !== 'loading') return;
 
     AutoPrefix.set(wrapper.style, 'transform', null);
     AutoPrefix.set(wrapper.style, 'transform', 'rotate(0deg)');
     AutoPrefix.set(wrapper.style, 'transitionDuration', '0ms');
 
-    setTimeout(() => {
-      if (this.isMounted()) {
-        AutoPrefix.set(wrapper.style, 'transform', 'rotate(1800deg)');
-        AutoPrefix.set(wrapper.style, 'transitionDuration', '10s');
-        AutoPrefix.set(wrapper.style, 'transitionTimingFunction', 'linear');
-      }
+    this.rotateWrapperSecondTimer = setTimeout(() => {
+      AutoPrefix.set(wrapper.style, 'transform', 'rotate(1800deg)');
+      AutoPrefix.set(wrapper.style, 'transitionDuration', '10s');
+      AutoPrefix.set(wrapper.style, 'transitionTimingFunction', 'linear');
     }, 50);
+
+    this.rotateWrapperTimer = setTimeout(() => this._rotateWrapper(wrapper), 10050);
   },
 
   prefixed(key) {

diff --git a/src/snackbar.jsx b/src/snackbar.jsx
@@ -21,11 +21,9 @@ const Snackbar = React.createClass({
 
   manuallyBindClickAway: true,
 
-  _timerAutoHideId: undefined,
-
-  _timerTransitionId: undefined,
-
-  _timerOneAtTheTimeId: undefined,
+  timerAutoHideId: undefined,
+  timerTransitionId: undefined,
+  timerOneAtTheTimeId: undefined,
 
   contextTypes: {
     muiTheme: React.PropTypes.object,
@@ -157,8 +155,8 @@ const Snackbar = React.createClass({
         open: false,
       });
 
-      clearTimeout(this._timerOneAtTheTimeId);
-      this._timerOneAtTheTimeId = setTimeout(() => {
+      clearTimeout(this.timerOneAtTheTimeId);
+      this.timerOneAtTheTimeId = setTimeout(() => {
         this.setState({
           message: nextProps.message,
           action: nextProps.action,
@@ -181,7 +179,7 @@ const Snackbar = React.createClass({
       this._setAutoHideTimer();
 
       //Only Bind clickaway after transition finishes
-      this._timerTransitionId = setTimeout(() => {
+      this.timerTransitionId = setTimeout(() => {
         this._bindClickAway();
       }, 400);
     }
@@ -201,20 +199,20 @@ const Snackbar = React.createClass({
         this._setAutoHideTimer();
 
         //Only Bind clickaway after transition finishes
-        this._timerTransitionId = setTimeout(() => {
+        this.timerTransitionId = setTimeout(() => {
           this._bindClickAway();
         }, 400);
       } else {
-        clearTimeout(this._timerAutoHideId);
+        clearTimeout(this.timerAutoHideId);
         this._unbindClickAway();
       }
     }
   },
 
   componentWillUnmount() {
-    clearTimeout(this._timerAutoHideId);
-    clearTimeout(this._timerTransitionId);
-    clearTimeout(this._timerOneAtTheTimeId);
+    clearTimeout(this.timerAutoHideId);
+    clearTimeout(this.timerTransitionId);
+    clearTimeout(this.timerOneAtTheTimeId);
     this._unbindClickAway();
   },
 
@@ -355,8 +353,8 @@ const Snackbar = React.createClass({
     const autoHideDuration = this.props.autoHideDuration;
 
     if (autoHideDuration > 0) {
-      clearTimeout(this._timerAutoHideId);
-      this._timerAutoHideId = setTimeout(() => {
+      clearTimeout(this.timerAutoHideId);
+      this.timerAutoHideId = setTimeout(() => {
         if (this.props.open !== null && this.props.onRequestClose) {
           this.props.onRequestClose('timeout');
         } else {