3

I tried to make a nice animating example for myself using Hooks but I have stumbled upon a problem where my functions won't have the updated version of a state var and continue to use the first version.

In the snippet below I have an example where once you click on a bar it's supposed to move in a square formation. First going east, then south, then west, then north and then east again etc. However it never goes south because even though its direction is updated from north to east (indicated by the text on the bar or clicking on it again), the functions still think that the direction is north.

const Example = (props) => {
  const [ direction, setDirection ] = React.useState('North');
  console.log("Rendering Example: ", direction);
  
  const isAnimating = React.useRef(false)
  const blockRef = React.useRef(null);
  
  // Animate on click.
  const onClick = () => {
    if (!isAnimating.current) {
      decideDirection();
      isAnimating.current = true
    } else {
      console.log("Already animating. Going: ", direction);
    }
  };
  
  const decideDirection = () => {
    console.log("Current direction: ", direction);
    if (direction === 'North') {
      moveEast();
    } else if (direction === 'East') {
      moveSouth();
    } else if (direction === 'South') {
      moveWest();
    } else if (direction === 'West') {
      moveNorth();
    }
  };
  
  const move = (toX, toY, duration, onComplete) => {
    Velocity(blockRef.current, {
      translateX: toX,
      translateY: toY,
      complete: () => {
        onComplete();
      }
    },
    {
      duration: duration
    });
  }
  
  const moveNorth = () => {
    setDirection('North');
    console.log('Moving N: ', direction);
    move(0, 0, 500, () => {
      decideDirection();
    })
  }
  
  const moveEast = () => {
    setDirection('East');
    console.log('Moving E: ', direction);
    move(500, 0, 2500, () => {
      decideDirection();
    })
  };
  
  const moveSouth = () => {
    setDirection('South');
    console.log('Moving S: ', direction);
    move(500, 18, 500, () => {
      decideDirection();
    })
  }
  
  const moveWest = () => {
    setDirection('West');
    console.log('Moving W: ', direction);
    move(0, 18, 2500, () => {
      decideDirection();
    })
  }
  
  return(
    <div>
      <div id='block' onClick={onClick} ref={blockRef} style={{ width: '100px', height: '18px', backgroundColor: 'red', textAlign: 'center'}}>{direction}</div>
    </div>
  );
};

ReactDOM.render(<div><Example/></div>, document.getElementById('root'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/velocity/1.2.2/velocity.min.js"></script>

<script src="https://cdn.jsdelivr.net/npm/lodash@4.17.11/lodash.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.3/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.3/umd/react-dom.production.min.js"></script>

<div id='root' style='width: 100%; height: 100%'>
</div>

I find this kind of strange because none of these functions are memoized so they should re-create every render and thus have the new value. Even if I do add something like useCallback and provide direction to every function it still won't work. Why don't the functions know about the updated version of the state var?

ApplePearPerson
  • 4,209
  • 4
  • 21
  • 36
  • 2
    I think this article can help you clarify https://overreacted.io/how-are-function-components-different-from-classes/ The state on the functions when you don't use classes is different, – Dvid Silva Mar 08 '19 at 16:04
  • 1
    Thank you for the article this explains a lot! Although I disagree with the writer that classes are the wrongdoer here – ApplePearPerson Mar 08 '19 at 16:21

2 Answers2

4

The issue is that your move functions have closed over the initial version of decideDirection. Your div has re-rendered, but the animation continues with references to the initial versions of the functions. One way to resolve this is to use a ref to point at the decideDirection function (I've shown this approach in the snippet below).

This seems a little brittle though since the timing of the re-render vs. the timing of the animation makes it difficult to reason as to whether this is robust. A more robust approach would involve chaining the moves more explicitly via state changes such that rather than directly calling decideDirection at completion of each animation, you instead set state that will trigger an effect to start the next animation.

const Example = (props) => {
  const [ direction, setDirection ] = React.useState('North');
  console.log("Rendering Example: ", direction);
  
  const isAnimating = React.useRef(false)
  const blockRef = React.useRef(null);
  const decideDirection = () => {
    console.log("Current direction: ", direction);
    if (direction === 'North') {
      moveEast();
    } else if (direction === 'East') {
      moveSouth();
    } else if (direction === 'South') {
      moveWest();
    } else if (direction === 'West') {
      moveNorth();
    }
  };
  const decideDirectionRef = React.useRef(decideDirection);
  React.useEffect(()=>{
    decideDirectionRef.current = decideDirection;
  });
  
  // Animate on click.
  const onClick = () => {
    if (!isAnimating.current) {
      decideDirectionRef.current();
      isAnimating.current = true
    } else {
      console.log("Already animating. Going: ", direction);
    }
  };
  
  const move = (toX, toY, duration, onComplete) => {
    Velocity(blockRef.current, {
      translateX: toX,
      translateY: toY,
      complete: () => {
        onComplete();
      }
    },
    {
      duration: duration
    });
  }
  
  const moveNorth = () => {
    setDirection('North');
    console.log('Moving N: ', direction);
    move(0, 0, 500, () => {
      decideDirectionRef.current();
    })
  }
  
  const moveEast = () => {
    setDirection('East');
    console.log('Moving E: ', direction);
    move(500, 0, 2500, () => {
      decideDirectionRef.current();
    })
  };
  
  const moveSouth = () => {
    setDirection('South');
    console.log('Moving S: ', direction);
    move(500, 18, 500, () => {
      decideDirectionRef.current();
    })
  }
  
  const moveWest = () => {
    setDirection('West');
    console.log('Moving W: ', direction);
    move(0, 18, 2500, () => {
      decideDirectionRef.current();
    })
  }
  
  return(
    <div>
      <div id='block' onClick={onClick} ref={blockRef} style={{ width: '100px', height: '18px', backgroundColor: 'red', textAlign: 'center'}}>{direction}</div>
    </div>
  );
};

ReactDOM.render(<div><Example/></div>, document.getElementById('root'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/velocity/1.2.2/velocity.min.js"></script>

<script src="https://cdn.jsdelivr.net/npm/lodash@4.17.11/lodash.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.3/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.3/umd/react-dom.production.min.js"></script>

<div id='root' style='width: 100%; height: 100%'>
</div>
Ryan Cogswell
  • 75,046
  • 9
  • 218
  • 198
  • Yeah I agree that there are a ton of better ways to write it, I was just really confused why this particular approach wasn't working. Thanks! – ApplePearPerson Mar 08 '19 at 16:23
  • While not a bad answer, I think it would be much more succinct to simply say your function has a value capture bug as @ygweric says below. – Matt Jensen Jun 19 '23 at 09:03
0

your value is captured, have a look about my another answer

dmathisen
  • 2,269
  • 3
  • 36
  • 63
ygweric
  • 1,002
  • 1
  • 7
  • 22
  • Very helpful answer, but it could be more helpful if it included a code snippet of the specific issue. Also, I'd move your answer to this page because I think that link is a duplicate of this page (already flagged it). – Matt Jensen Jun 19 '23 at 09:09