I believe what you're doing here in an anti-pattern and should be avoided altogether.
In the first code snippet, instead of the category itself, storing only the index in state is enough to avoid the problem of a potentially stale category when the props change.
Generally, you don't want to compute the state from the props, that's error prone and most of the time not needed.
Furthermore, useMemo should be used to compute a memoized value and not to perform side-effects, for that purpose useEffect should be used.
The official doc on hooks is very clear about it.
Also, setCategory(category = result[0]) and setState(state = newState): that's not how state updaters should be called. If anything, setCategory(result[0]) and setState(newState) would be correct. The category and state variables shouldn't be reassigned (using const instead of let will enforce that).
Hi Guico33, thanks for your input. I considered your suggested approach while writing the article, but storing the selected index in state does not avoid the problem. In fact, it makes the problem worse. Consider:
Here, when options changes, selectedIndex may no longer point to a valid entry in the options array at all, or it may point to a completely different item, which will result in the UI abruptly changing to a different category. You cannot fix this, because you cannot know what item selectedIndex pointed to during previous renders. That is why you have to keep the item itself as piece of state as well.
I understand your points re. running side effects in useMemo and reassigning the state variables, but so long as this is contained within the useDependentState custom hook, it shouldn't pose any danger. We could use useEffect instead of useMemo, except that it would result in a first-pass render with a stale value, which might or might not be acceptable, because the useEffect callback does not run until after the render has completed. If you're concerned with not breaking the "rules" of hooks, you can write the custom hook as follows:
However, this does have the problem of producing a stale render, as mentioned. Personally, I'm fine with the ignoring the "rules", if I'm doing so consciously, and in a way I know won't break anything.
There's a reason whileuseMemo shouldn't be used, it runs while rendering, thus updating the state in it might not trigger a rerender and result in a stale UI.
I agree using the index may not be a safe choice, a better solution would be to use an id of some sort.
If the corresponding item is no longer there, you can reset the state to the id of the first item for instance, in a useEffect hook.
Only gotcha is if you want to keep track of an item which is no longer present in the data passed as props. Assuming the data is a list of options fetched remotely, it's unlikely you want the user to select an item which is no longer in the list returned by the server.
Even with an id, you still have the issue that, for that render, the id state value is no longer valid. This means you have to deal with checking the validity of the id twice, both in the effect, and in the render method, and the resulting code is pretty ugly and unintuitive, IMO:
In this case, the fact that useMemo runs immediately is to our benefit, because we do want to update the state value immediately, because otherwise we're going to render invalid data. By both calling the state setter and reassigning the current state value, we avoid any problems with either invalid or stale renders. And wrapping this logic up inside a custom hook ensures that the mutability of the state variable is never leaked to the outside code.
I agree this isn't a 100% ideal solution, but I have used it and it works, without any issues regarding stale UI. However, it would be nicer if useState supported these kind of dependencies directly, which is why I raised the feature request I mentioned with the React team.
selectedOption may not be defined so you need to account for that in your render method, but that's about the same as every time you're fetching data in a react component.
The computation of the options is a good use case for useMemo. selectedOption could use it as well, though for both it remains an optimisation and wouldn't make a difference in most cases.
This code will render with selectedOption as null for the render before the one scheduled by the effect. What if you can't do this? You will need to add something like:
In my previous comment: selectedOption may not be defined so you need to account for that in your render method.
Defaulting it to the first option works as well. One could argue that the source of truth should be the id in state so you might wanna wait until the state has been updated and until then render something else (likely all this will be too fast to be noticeable by the user), though in the end I guess it's about the same.
I don't know whether this is still an anti-pattern or not, but i'm trying to ship an MVP and this was the answer I needed to get it to work, so thank you!
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
I believe what you're doing here in an anti-pattern and should be avoided altogether.
In the first code snippet, instead of the category itself, storing only the index in state is enough to avoid the problem of a potentially stale category when the props change.
Generally, you don't want to compute the state from the props, that's error prone and most of the time not needed.
Furthermore,
useMemo
should be used to compute a memoized value and not to perform side-effects, for that purposeuseEffect
should be used.The official doc on hooks is very clear about it.
Also,
setCategory(category = result[0])
andsetState(state = newState)
: that's not how state updaters should be called. If anything,setCategory(result[0])
andsetState(newState)
would be correct. Thecategory
andstate
variables shouldn't be reassigned (usingconst
instead oflet
will enforce that).Hi Guico33, thanks for your input. I considered your suggested approach while writing the article, but storing the selected index in state does not avoid the problem. In fact, it makes the problem worse. Consider:
Here, when
options
changes,selectedIndex
may no longer point to a valid entry in the options array at all, or it may point to a completely different item, which will result in the UI abruptly changing to a different category. You cannot fix this, because you cannot know what itemselectedIndex
pointed to during previous renders. That is why you have to keep the item itself as piece of state as well.I understand your points re. running side effects in
useMemo
and reassigning the state variables, but so long as this is contained within theuseDependentState
custom hook, it shouldn't pose any danger. We could useuseEffect
instead ofuseMemo
, except that it would result in a first-pass render with a stale value, which might or might not be acceptable, because theuseEffect
callback does not run until after the render has completed. If you're concerned with not breaking the "rules" of hooks, you can write the custom hook as follows:However, this does have the problem of producing a stale render, as mentioned. Personally, I'm fine with the ignoring the "rules", if I'm doing so consciously, and in a way I know won't break anything.
There's a reason while
useMemo
shouldn't be used, it runs while rendering, thus updating the state in it might not trigger a rerender and result in a stale UI.I agree using the index may not be a safe choice, a better solution would be to use an id of some sort.
If the corresponding item is no longer there, you can reset the state to the id of the first item for instance, in a
useEffect
hook.Only gotcha is if you want to keep track of an item which is no longer present in the data passed as props. Assuming the data is a list of options fetched remotely, it's unlikely you want the user to select an item which is no longer in the list returned by the server.
Even with an id, you still have the issue that, for that render, the id state value is no longer valid. This means you have to deal with checking the validity of the id twice, both in the effect, and in the render method, and the resulting code is pretty ugly and unintuitive, IMO:
In this case, the fact that
useMemo
runs immediately is to our benefit, because we do want to update the state value immediately, because otherwise we're going to render invalid data. By both calling the state setter and reassigning the current state value, we avoid any problems with either invalid or stale renders. And wrapping this logic up inside a custom hook ensures that the mutability of the state variable is never leaked to the outside code.I agree this isn't a 100% ideal solution, but I have used it and it works, without any issues regarding stale UI. However, it would be nicer if
useState
supported these kind of dependencies directly, which is why I raised the feature request I mentioned with the React team.The code does not need to be so verbose, it boils down to:
selectedOption
may not be defined so you need to account for that in yourrender
method, but that's about the same as every time you're fetching data in a react component.The computation of the options is a good use case for
useMemo
.selectedOption
could use it as well, though for both it remains an optimisation and wouldn't make a difference in most cases.This code will render with
selectedOption
as null for the render before the one scheduled by the effect. What if you can't do this? You will need to add something like:In my previous comment:
selectedOption may not be defined so you need to account for that in your render method.
Defaulting it to the first option works as well. One could argue that the source of truth should be the id in state so you might wanna wait until the state has been updated and until then render something else (likely all this will be too fast to be noticeable by the user), though in the end I guess it's about the same.
I don't know whether this is still an anti-pattern or not, but i'm trying to ship an MVP and this was the answer I needed to get it to work, so thank you!