I recently came across a piece of React code that caught my attention (new project, new codebase and this pattern is used quite a lot).
Imagine you have a table displaying a list of movies and you can select each movie row by ticking a checkbox. The checkbox selection needs to reset every time the user selects a different year, category or rating somewhere else on the page (the table needs to display new data when these filters are changed, so the selection is no longer valid).
The code I am talking about was a useEffect that was setting the selectedMovieId state and it was written something like:
const [selectedMovieId, setSelectedMovieId] = useState<string>("")
// more code here...
useEffect(() => {
setSelectedMovieId(null)
}, [
selectedYear,
selectedCategory,
selectedUserRating
])
I looked at the code for a few seconds before understanding that the developer was trying to update the state by providing extra dependencies to useEffect instead of doing that inside the event handlers of the year, category and user rating filters.
I would have written the code differently, making the selectedMovieId value get changed by functions, which themselves would be the source of the mutation and remove the useEffect.
const handleYearChange = (year: YearType) => {
setSelectedYear(year)
// something something...
setSelectedMovieId(null)
}
const handleCategoryChange = (category: CategoryType) => {
setSelectedCategory(category)
// something something...
setSelectedMovieId(null)
}
const handleRatingChange = (rating: RatingType) => {
setSelectedUserRating(rating)
// something something...
setSelectedMovieId(null)
}
While the original code is working, if in the future more filters will be added, we risk that the useEffect will never be updated with the extra dependencies, since this piece of logic can be easily missed.
Would you use this pattern? If yes, when?
Top comments (0)