Общие комментарии

1. Используйте TypeScript, на современном рынке даже джуну уже не достаточно примитивных знаний TS — большинство компаний требуют хороший опыт работы с типами, понимание дженериков и прочих не самых примитивных тем

2. Выносите хотя бы ёмкие константы куда-то из файла компонента, чтобы до самого компонента не приходилось листать 100+ строк при открытии файла

3. Выносите из компонента все независимые от его состояния сущности — в константы, в утилсы и в любое другое подходящее место. Это упростит чтение кода и зафиксирует ссылки

4. Не стесняйтесь создавать свои хуки, это ок. Есть примеры, где вызов useEffect достигает 50+ строк. Вынесите/разбейте, ничего страшного в этом нет

5. Очень много непонятных закомментированных частей кода в некоторых проектах. Комментируете = не надо = удаляете, восстановить всегда можно через гит

6. Используйте index файлы для организации импортов, это делает импорты короче и читабельнее, а также позволяет проще управлять интерфейсом части кода. Используя index файл, вы явно определяете что можно импортировать, а что нельзя. Подробнее на доке

7. Аккуратнее с инлайн стилями. Их можно использовать, но очень аккуратно и в крайних случаях. ИМХО лучше !important поставить, чем инлайнить. Ну а если используйте, то не пишите стили прямо в вёрстке. 99% случаев, которые я видел, не зависят от состояния компонента, так что можно легко создать константу вне компонента и зафиксировать ссылку на объект, что может быть очень полезно и в целом разгрузит вёрстку

8. Не используйте классовые компоненты, если в этом нет явной необходимости. Мир функциональных компонентов уже непобедим

9. Не храните ключи и прочие переменные в коде, используйте переменные окружения. Из кода можно легко компрометировать все ваши ключи, тем более проекты лежат в открытом доступе

10. Не нужно оборачивать в memo, useMemo, useCallback абсолютно всё. В ревью видел много мест, где memo используется там, где компонент имеет 1 рендер за весь жизненный цикл и без него, а useCallback скорее замедляет приложением, чем оптимизирует

11. Можно смириться с одним тернарным оператором в вёрстке, но когда их в одной конструкции сразу два… три… такое мы рефакторим обычными if-ами)

Отдельные посты по этому ревью будут выходить с завтрашнего дня

@prog_way_blog — чат — #review

Использование gap вместо margin

Начнем комментарии по проектам с популярной ошибки — использование margin вместо gap. Посты постараюсь делать покороче, не хочу развозить на это всю ленту.

Почему использовать gap лучше margin:
1. Легче изменять код — чаще всего отступы везде одинаковые в таких списках. При изменении отступов, проще изменить его в одном месте, чем для каждого компонента вёрстки
2. Более чистый код, отсутствие дублирования стилей
3. Лучше производительность — браузеру куда проще обработать gap в структурированной сетке, чем нарисовать для каждого отдельного элемента собственный отступ

Что такое свойство gap
Что такое row-gap, column-gap

@prog_way_blog — #web #review

Файловая структура проекта

Очень важно понимать, что файловая структура — это самый верный помощник разработчика в навигации по проекту.

В некоторых проектах, что прислали на ревью, в файловой структуре бывало достаточно тяжело разобраться. Тут и pages внутри components, и другие чудеса.

Чтобы путаницы не возникало при переходе от проекта к проекту, стоит использовать общепринятые нормы оформления проектов, которые под собой подразумевают в том числе организованную файловую структуру. Для примера, это может быть FSD, который сейчас на пике популярности.

Также стоит руководствоваться просто логикой. Лично для меня не до конца понятно, когда в папку utils складывают компоненты и валидаторы, а константы в папку services

Компоненты я бы сложил к компонентам, даже если они утилитарные (пример на фото), а валидаторы в сервисы, хотя момент всё таки спорный. Лично я бы сделал что-то типа ValidatorService и разделил бы его на два дочерних сервиса: InputValidator и ModelValidator

Тут InputValidator отвечал бы, очевидно, за ввод, а ModelValidator для валидации моделей, например, валидации payload’a из ответа апишки.

Вызов самого валидатора был бы следующий:

 ValidatorService.InputValidator.email(email)

Я люблю такие оргструктуры, это мой код стайл. Кому-то он покажется странным, но его плюсы очевидны:

1. Строгая иерархичность и однозначность расположения валидаторов
2. Простой поиск того, что тебе нужно
3. Красивые импорты
4. Простое управление публичным интерфейсом сущности

Архитектурная методология Feature Slices Design

@prog_way_blog — #review

Нейминг файлов

1. Определитесь в каком стиле именуете файлы. В одном проекте не должно существовать файлов MarktetTrends.jsx, filterSlice.js, table.hook.jsx. Если делаем групповой суффикс к файлу, типа *.component.jsx, то делаем везде. Если используем camelCase, то везде. Выглядит симпатичнее, искать проще, нет визуальной каши.

2. Я советовал бы переименовать table.hook.jsx в use-table.hool.jsx или use-table.jsx. Вы же знаете, что с use начинаются хуки. В поиске инстинктивно напишете use table и файл table.hook.jsx просто не найдёте. Неймингом можно спасти не одну нервную клетку, так же как и убить.

3. В названии файлов стоит избегать аббревиатур и сокращений, всегда удивляла мания экономить буквы у разработчиков. Лучше использовать только общепринятые сокращения, например, http , чтобы избежать недопониманий

@prog_way_blog — #review

Линтеры

Я мог бы кучу моментов подметить на тему того, что “тут кавычки не те”, а “тут отступы кривые”. Настройте линтер. На подобные комменты в процессе ревью вообще отвлекаться — антипаттерн. Если ревью состоит из того, что вам указывают где какую кавычку лучше поставить, то это бред, а не ревью. Подобные вопросы должны решаться автоматизировано.

Другая проблема — в некоторых проектах есть eslint/prettier, супер, круто, но видно же, что код отформатирован не по ним, а в некоторых местах вообще стоят eslint-disable комменты. Пожалуйста, помимо закидывания конфиги в проект, убедитесь, что линтер реально работает и форматирует всё так, как ожидается. Ну и eslint-disable использовать тоже очевидно не нужно)

@prog_way_blog — #review

Комплексные состояния

Есть такой код:

const [userData, setUserData] = React.useState<IData>({
  identifier: "",
  password: "",
});

Как его можно исправить:

const [identifier, setIdentifier] = useState<string>("");
const [password, setPassword] = useState<string>("");

Почему я считаю что так лучше:
1. Проще следить за иммутабельностью состояния, так как не нужно постоянно разворачивать prev объект
2. Легче контролировать зависимости и сайд эффекты, если на состояние завязывается, например, useEffect
3. Визуально в использовании считается легче
4. useState вместо React.useState, ИМХО приятнее
5. Нет необходимости создавать ещё какой-то тип

В целом, можно и так оставить то. Работать будет, и даже уже сейчас работает. Но лично мне такое бьет по глазам.

@prog_way_blog —#typescript #web #review

Магические числа

Немного магии — не всегда приятно. Есть код:

date.setTime(date.getTime() + (365 * 24 * 60 * 60 * 1000));

Как можно сделать:

const MILLISECONDS_PER_YEAR = 365 * 24 * 60 * 60 * 1000;
date.setTime(date.getTime() + MILLISECONDS_PER_YEAR);

Как на самом деле нужно сделать:

const date = new Date();
date.setFullYear(date.getFullYear() + 1);

Так нужно сделать, чтобы не завязываться на число 365, так как год бывает високосным.

@prog_way_blog — #review

Ответственность состояния

Есть код:

const [isFormSubmited, setFormSubmited] = React.useState({
  submited: false,
  submitedText: "",
  submitedError: "",
  submitedColor: "",
});


setFormSubmited({
  submited: true,
  submitedText: "Вы успешно зарегистрировались!",
  submitedError: "",
  submitedColor: "#238C47",
});

Как можно было бы сделать:

enum FormState {
 Success,
 Error,
 WaitingForAction
}

const [formState, setFormState] = useState<FormState>(FormState.WaitingForAction);

Ну и в зависимости от formState рисовать любую фразу любого цвета, которого хочется. В этом случае вообще не понятно почему цвет текста и текст сообщения были вынесены в состояние. Оставьте это в вёрстке, зачем в состояние то.

@prog_way_blog — #review

Страшные рефы

Есть код:

const items = [1, 2, 3, 4];
const itemRefs: React.RefObject<HTMLDivElement>[] = items.map(() =>
  useRef(null),
);

// и далее в вёрстке
ref={itemRefs[index]}

Как сделал бы я:

// вынести из компонента
const items = [1, 2, 3, 4];

// внутри компонента
const itemRefs = useRef([])

// и внутри вёрстки
// index берется из .map, так как элементы на 
// страницу вставляются из списка
ref={element = inputRef.current[index] = element}

Цитата из доки реакта: “Не вызывайте хуки внутри циклов, условных операторов или вложенных функций. Вместо этого всегда используйте хуки только внутри React-функций, до возврата какого-либо значения из них.”. Если это правило не соблюдать, то можно самых странных ошибок огрести.

Подробнее в официальном руководстве

@prog_way_blog — #react #review

Зачем нужен useCallback

Очень распространенная ошибка, которая говорит о том, что мало кто понимает что такое useCallback и зачем он нужен.

Важный вопрос, будет ли ререндерится Component при ререндере родителя?

const Parent = () => {
 const foo = useCallback(() => {
  
 }, [])
 
 return <Component foo={foo} />
}

const Component = (props) => { ... }

И ответ тут очевиден — конечно же да! Не смотря на то, что функцию мы мемоизировали и ссылка на саму функцию у нас не изменилась благодаря useCallback, мы всё равно перерендерим Component, поскольку рендерится родитель.

Чтобы получить ожидаемое поведение, необходимо сделать очень важную вещь, а именно — мемоизировать и сам дочерний компонент:

const Component = memo((props) => { ... })

Вот только так и только тогда мы перестанем ререндерить дочерние компоненты, так как пропсы не изменились. При ревью я заметил много случаев, когда useCallback использовать просто не нужно было, но он был.

А ещё много таких было кейсов:

const Component = () => {
 const foo = useCallback(() => {
  
 }, [])
 
 return <button onClick={() => foo()} />
}

Тут смысл useCallback теряется вдвойне, так как в пропсах мы всё равно пересоздаём функцию на каждый рендер вот тут: onClick={() => foo()}

Пример из доки реакта

@prog_way_blog — #react #review