I'm not certain, but it looks like you have a problem with a stale callback here.
useEffect(() => {
const interval = setInterval(counterIntervalFunction, props.speed);
setRowState({ ...rowState, renderInterval: interval });
}, []);
This effect only runs once - When the component is mounted the first time. It uses the counterIntervalFunction function for the interval:
const counterIntervalFunction = () => {
if (props.isRunning && props.direction === 'ltr') {
const ltrNewValue = rowState.value === 2 ? 0 : rowState.value + 1;
console.log(ltrNewValue); // always 1
setRowState({ ...rowState, value: ltrNewValue });
console.log(rowState.value); // always 0
props.setRotatingValue(props.index, rowState.value);
} else if (props.isRunning && props.direction === 'rtl') {
const rtlNewValue = rowState.value === 0 ? 2 : rowState.value - 1;
setRowState({ ...rowState, value: rtlNewValue });
props.setRotatingValue(props.index, rowState.value);
} else {
clearCounterInterval();
}
};
The counterIntervalFunction captures the reference of props and uses it to determine what to display to the user. However, because this function is only run when the component is mounted, the event will only be run with the props passed to the function originally! You can see an example of this happening in this codesandbox.io
This is why you should put all external dependencies inside of the dependencies array:
useEffect(() => {
const interval = setInterval(counterIntervalFunction, props.speed);
setRowState({ ...rowState, renderInterval: interval });
}, [counterIntervalFunction, props.speed, rowState]);
However, this will cause an infinite loop.
Setting state in useEffect is usually considered a bad idea, because it tends to lead to infinite loops - changing the state will cause the component to re-render, causing another effect to be triggered etc.
Looking at your effect loop, what you're actually interested in is capturing a reference to the interval. This interval won't actually have any impact on the component if it changes, so instead of using state, we can use a ref to keep track of it. Refs don't cause re-renders. This also means we can change value to be a stand-alone value.
Because we now no longer depend on rowState, we can remove that from the dependencies array, preventing an infinite render. Now our effect only depends on props.speed and counterIntervalFunction:
const renderInterval = React.useRef();
const [value, setValue] = React.useState(0);
useEffect(() => {
renderInterval.current = setInterval(counterIntervalFunction, props.speed);
return () => {
cancelInterval(renderInterval.current);
};
}, [props.speed, counterIntervalFunction]);
This will work, but because counterIntervalFunction is defined inline, it will be recreated every render, causing the effect to trigger every render. We can stablize it with React.useCallback(). We'll also want to add all the dependencies of this function to ensure that we don't capture stale references to props and we can change setRowState to setValue. Finally, because the interval is cancelled by useEffect, we don't need to call clearCounterInterval anymore.
const counterIntervalFunction = React.useCallback(() => {
if (props.isRunning && props.direction === 'ltr') {
const ltrNewValue = value === 2 ? 0 : value + 1;
setValue(ltrNewValue);
props.setRotatingValue(props.index, ltrNewValue);
} else if (isRunning && props.direction === 'rtl') {
const rtlNewValue = value === 0 ? 2 : value - 1;
setValue(rtlNewValue);
props.setRotatingValue(props.index, rtlNewValue);
}
}, [value, props]);
This can be simplified even further by moving the required props to the arguments:
const counterIntervalFunction = React.useCallback((isRunning, direction, setRotatingValue, index) => {
if (isRunning === false) {
return;
}
if (direction === 'ltr') {
const ltrNewValue = value === 2 ? 0 : value + 1;
setValue(ltrNewValue);
setRotatingValue(index, ltrNewValue);
} else if (props.direction === 'rtl') {
const rtlNewValue = value === 0 ? 2 : value - 1;
setValue(rtlNewValue);
setRotatingValue(index, rtlNewValue);
}
}, [value]);
This could be even simpler if not for setRotatingValue: Right now, you have a component that both maintains it's own state and tells the parent when its state changes. You should be aware that the component state value might not necessarily update when you call it, but setRotatingValue absolutely will. This may lead to a situation where the parent sees a different state than the child does. I would recommend altering the way your data flows such that it's the parent that owns the current value and passes it via props, not the child.
This gives us the following code to finish off:
function Row = (props) => {
const renderInterval = React.useRef();
const [value, setValue] = React.useState(0);
useEffect(() => {
renderInterval.current = setInterval(counterIntervalFunction, props.isRunning, props.direction, props.setRotatingValue, props.index);
return () => {
cancelInterval(renderInterval.current);
};
}, [props, counterIntervalFunction]);
const counterIntervalFunction = React.useCallback((isRunning, direction, setRotatingValue, index) => {
if (isRunning === false) {
return;
}
if (direction === 'ltr') {
const ltrNewValue = value === 2 ? 0 : value + 1;
setValue(ltrNewValue);
setRotatingValue(index, ltrNewValue);
} else if (props.direction === 'rtl') {
const rtlNewValue = value === 0 ? 2 : value - 1;
setValue(rtlNewValue);
setRotatingValue(index, rtlNewValue);
}
}, [value]);
...
}
In this code, you'll notice that we run the effect every time the props or the function changes. This will mean that, unfortunately, the effect will return every loop, because we need to keep a fresh reference to value. This component will always have this problem unless you refactor counterIntervalFunction to not notify the parent with setRotatingValue or for this function to not contain its own state. An alternatively way we could solve this would be using the function form of setValue:
const counterIntervalFunction = React.useCallback((isRunning, direction, setRotatingValue, index) => {
if (isRunning === false) {
return;
}
setValue(value => {
if (direction === 'ltr') {
return value === 2 ? 0 : value + 1;
} else if (direction ==' rtl') {
return value === 0 ? 2 : value - 1;
}
})
}, []);
Because the state update is not guaranteed to run synchronously, there's no way to extract the value from the setValue call and then call the setRotatingValue function, though. :( You could potentially call setRotatingValue inside of the setValue callback but that gives me the heebie geebies.
