2

How would one update the value of variable simulationOn inside of function executeSimulation in the following context:

App this.state.simulationOn changes via external code --> ... --> React stateless component (Robot) rerendered --> useEffect hook called with new values --> executeSimulation IS NOT UPDATED with new value of simulationOn.

    function Robot({ simulationOn, alreadyActivated, robotCommands }) {

        useEffect(() => {
            function executeSimulation(index, givenCommmands) {
                index += 1;
                if (index > givenCommmands.length || !simulationOn) {
                    return;
                }
                setTimeout(executeSimulation.bind({}, index, givenCommmands), 1050);
            }
            if (simulationOn && !alreadyActivated) {
                executeSimulation(1, robotCommands);
            }
        }, [simulationOn, alreadyActivated, robotCommands]);

    }

In the example above, simulationOn never changes to false, even though useEffect is called with the updated value (I check with console.log). I suspect this is because the new value of simulationOn is never passed to the scope of function executeSimulation, but I don't know how to pass new hook values inside of function executeSimulation.

  • simulationOn changes where? – zhuber Mar 21 '20 at 09:54
  • Sorry, I just made the question clearer! `this.state.simulationOn` changes in the App class via external code. – Theodor Amariucai Mar 21 '20 at 09:58
  • I really don't understand which part is the problem? If you want to manually change simulationOn, you have to pass setSimulationOn callback all the way to the component where you want to change it. If you have problems with simulation still occuring even if simulationOn=false, then the problem might be because you're not clearing setTimeout. – zhuber Mar 21 '20 at 10:06
  • I am not trying to manually change `simulationOn`, that already happens. I am trying to make function `executeSimulation` aware of changes made to hook variables (specifically `simulationOn`) while it's running. Edit: tried clearing setTimeout but again this is just ignored. – Theodor Amariucai Mar 21 '20 at 10:10
  • 1
    Why does `executeSimulation` have to be in the scope of the useEffect? If you want it to reference the current value of `simulationOn` on each timeout you should declare it in the scope of the component and reference that prop directly. `useEffect` effectively creates an instance for that render and that render alone, it is not possible to pass anything inside it new props - they will always be passed to a new instance. – lawrence-witt Mar 21 '20 at 11:05
  • Good question. Because my full `useEffect` code (linked in the question) contains code which updates the state hook of the component, which in turn triggers a re-render, which will start an infinite loop as indicated by the error `Error: Too many re-renders. React limits the number of renders to prevent an infinite loop.` By putting my code inside of `useEffect`, I discovered I could change state and not trigger an infinite loop, though I'm not entirely sure why. – Theodor Amariucai Mar 21 '20 at 12:13

2 Answers2

0

The executeSimulation function has a stale closure simulationOn will never be true, here is code demonstrating stale closure:

var component = test => {
  console.log('called Component with',test);
  setTimeout(
    () => console.log('test in callback:', test),
    20
  );
}
component(true);
coponent(false)

Note that Robot is called every time it renders but executeSimulation runs from a previous render having it's previous simulationOn value in it's closure (see stale closure example above)

Instead of checking simulationOn in executeSimulation you should just start executeSimulation when simulationOn is true and clearTimeout in the cleanup function of the useEffect:

const Component = ({ simulation, steps, reset }) => {
  const [current, setCurrent] = React.useState(0);
  const continueRunning =
    current < steps.length - 1 && simulation;
  //if reset or steps changes then set current index to 0
  React.useEffect(() => setCurrent(0), [reset, steps]);
  React.useEffect(() => {
    let timer;
    function executeSimulation() {
      setCurrent(current => current + 1);
      //set timer for the cleanup to cancel it when simulation changes
      timer = setTimeout(executeSimulation, 1200);
    }
    if (continueRunning) {
      timer = setTimeout(executeSimulation, 1200);
    }
    return () => {
      clearTimeout(timer);
    };
  }, [continueRunning]);
  return (
    <React.Fragment>
      <h1>Step: {steps[current]}</h1>
      <h1>Simulation: {simulation ? 'on' : 'off'}</h1>
      <h1>Current index: {current}</h1>
    </React.Fragment>
  );
};
const App = () => {
  const randomArray = (length = 3, min = 1, max = 100) =>
    [...new Array(length)].map(
      () => Math.floor(Math.random() * (max - min)) + min
    );
  const [simulation, setSimulation] = React.useState(false);
  const [reset, setReset] = React.useState({});
  const [steps, setSteps] = React.useState(randomArray());
  return (
    <div>
      <button onClick={() => setSimulation(s => !s)}>
        {simulation ? 'Pause' : 'Start'} simulation
      </button>
      <button onClick={() => setReset({})}>reset</button>
      <button onClick={() => setSteps(randomArray())}>
        new steps
      </button>
      <Component
        simulation={simulation}
        reset={reset}
        steps={steps}
      />
      <div>Steps: {JSON.stringify(steps)}</div>
    </div>
  );
};
ReactDOM.render(<App />, document.getElementById('root'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.4/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.4/umd/react-dom.production.min.js"></script>
<div id="root"></div>
HMR
  • 37,593
  • 24
  • 91
  • 160
  • Yes, thank you for pinpointing the issue and providing code too! This will not solve the problem because the cleanup function triggers every time a prop has changed, instead of only when `simulationOn` has changed. (my dependency array is not just `[simulationOn]` but much larger). I now think the answer might be related to https://stackoverflow.com/questions/57481573/react-useeffect-cleanup-with-current-props – Theodor Amariucai Mar 21 '20 at 12:30
  • @TheodorAmariucai I have updated the answer with the `App` component being able to start and pause simulation (with simulation), reset (with reset) and create new simulation steps (with steps). You can run the code and see if it does what you need it to do (click full page link to see all content) – HMR Mar 21 '20 at 13:23
0

simulationOn is never changed, because the parent component has to change it. It is passed in a property to this Robot component. I created a sandbox example to show, if you change it properly in the parent, it will propagate down. https://codesandbox.io/s/robot-i85lf.

There a few design issues in this robot. It seems assume a Robot can "remember" the index value by holding it as a instance variable. React doesn't work that way. Also, it assume that useEffect will be called exactly once when one parameter is changed, which is not true. We don't know how many times useEffect will be invoked. React only guarantee that it will be called if one of the dependencies are changed. But it could be invoked more often.

My example shows that a command list has to be kept by the parent, and the full list needs to be sent, so the dumb Robot can show all the commands it executed.

tigerpaw
  • 135
  • 4