Сделайте ревью этого компонента React: какие баги отметите?
Сделайте ревью компонента ниже. Найдите баги корректности и нарушения соглашений, объясните, почему каждое важно. Сосредоточьтесь на эффекте загрузки данных, обработке формы данных и отрисовке списка.
export const SpotList = (props) => {
const [spot, setSpot] = useState();
const [location, setLocation] = useState({});
useEffect(() => {
loadLocation(props.locationId).then((loc) => setLocation(loc));
});
const spotTimes = props.spots.map((s) => {
const slot = s.timeslots.find((t) => t.isAvailable);
return { [s.id]: slot };
});
return (
<>
<header>Spots for {location.name}</header>
{props.spots.map((s) => (
<div onClick={() => setSpot(s.id)} className={spot == s.id ? 'active' : ''}>
{s.id} at {spotTimes[s.id]}
</div>
))}
</>
);
};Найдите баги.
Главный баг: у useEffect нет массива зависимостей, поэтому он перезапрашивает на каждый рендер — а каждый setLocation вызывает новый рендер, фактически цикл; задайте [props.locationId]. spotTimes — это МАССИВ объектов с одним ключом, но читается как spotTimes[s.id] — неверная форма; постройте один объект или Map по id. У <div> в map нет key, что ломает согласование. Мелочи: === и состояние загрузки для location.
- ✗Упускать баг с отсутствием массива зависимостей, из-за которого эффект перезапрашивает на каждый рендер
- ✗Не замечать, что
spotTimes— массив объектов, но индексируется как карта по ключу - ✗Пропускать отсутствующий
keyу элементов списка изmap
- →Почему эффект без массива зависимостей вместе с
setStateсоздаёт цикл перезапросов? - →Как переформировать
spotTimes, чтобыspotTimes[s.id]читался верно?
Решение
Главные баги — эффект без зависимостей и неверная форма spotTimes.
export const SpotList = (props) => {
const [spot, setSpot] = useState();
const [location, setLocation] = useState(null);
useEffect(() => {
loadLocation(props.locationId).then(setLocation);
}, [props.locationId]); // зависимость: запрос лишь при смене id
const spotTimes = props.spots.reduce((acc, s) => {
acc[s.id] = s.timeslots.find((t) => t.isAvailable);
return acc;
}, {}); // один объект, читаемый по spotTimes[s.id]
if (!location) return <p>Loading…</p>;
return (
<>
<header>Spots for {location.name}</header>
{props.spots.map((s) => (
<div
key={s.id}
onClick={() => setSpot(s.id)}
className={spot === s.id ? 'active' : ''}
>
{s.id} at {spotTimes[s.id]?.label}
</div>
))}
</>
);
};Как это работает
useEffect без второго аргумента запускается после КАЖДОГО рендера. Внутри он зовёт setLocation, что вызывает новый рендер, который снова запускает эффект — получается лавина запросов. Массив зависимостей [props.locationId] ограничивает запуск сменой id.
spotTimes в оригинале — массив объектов вида { [id]: slot }, но затем читается как spotTimes[s.id], что почти всегда undefined. Правильно собрать один объект (или Map) по id через reduce. Плюс мелочи: у элементов map обязателен стабильный key для согласования, === вместо ==, и обработка состояния, пока location ещё не загружен.
