Some of the code reviews comments I usually write for junior React developers.
First let's take a look at this component:
const SimpleComponent = () => {
const [price, setPrice] = React.useState(10);
const [tax, setTax] = React.useState(0.15);
const [total, setTotal] = React.useState(price * tax);
React.useEffect(() => {
setTotal(price + price * tax);
}, [price, tax]);
return (
<div>
<input type="number" value={price} onChange={({ target: { value } }) => setPrice(+value)} />
<span>Total: {total}</span>
</div>
);
};
First Code Smell: a state setter setTax
is not used, which means tax
can be a variable defined out of the component scope, or maybe it's even better to keep it as an optional prop with a default value.
const SimpleComponent = ({ tax = 0.15 }) => {
const [price, setPrice] = React.useState(10);
const [total, setTotal] = React.useState(price * tax);
React.useEffect(() => {
setTotal(price + price * tax);
}, [price, tax]);
return (
<div>
<input type="number" value={price} onChange={({ target: { value } }) => setPrice(+value)} />
<span>Total: {total}</span>
</div>
);
};
There may be a special case where you want to keep the initial value of a prop, so you won't need to use the setter, which is fine.
Second Code Smell: A state setter is used only after another state changes.
React.useEffect(() => {
setTotal(price + price * tax);
}, [price]);
In this example, total
changes only after the price
state changes, which could be replace with a simple variable
const SimpleComponent = ({ tax = 0.15 }) => {
const [price, setPrice] = React.useState(10);
const total = price + price * tax;
return (
<div>
<input type="number" value={price} onChange={({ target: { value } }) => setPrice(+value)} />
<span>Total: {total}</span>
</div>
);
};
By this, we went down from 3 states to 1 state, which makes our component much easier to understand and read.
These may be simple notes, but they can slip in huge components like a data table. Where you should keep which column we are sorting by not the sorted rows, same for filters/pagination.
// Don't
const [sortedRows, setSortedRows] = React.useState(rows);
const handleSortByName = () => {
setSortedRows(sortRowsBy(rows, "name"));
}
// Do
const [sortBy, setSortBy] = React.useState(null);
const sortedRows = sortRowsBy(rows, sortBy);
const handleSortByName = () => {
setSortBy("name");
}
sortedRows
is what I call a "Computed State" as it changes based on another state.
If it makes performance issues you can always useMemo.
const sortedRows = React.useMemo(() => sortRowsBy(rows, sortBy), [rows, sortBy])
Hey, this is my first blog ever. And it's the first of a series. Please let me know if you have any notes/questions and if you want to see more of this series.
Top comments (0)