Review 18.08.2021

[bitvalser]

1) Пути лучше вынести в константы (core/constants/appRoutes.ts) <Route exact path='/signup' component={SignUpPage} /> Создать и использовать enum для этого AppRoutes.SignUp По поводу констант, вижу много где можно использовать, если есть switch case со строками, то это определенно должен быть создан enum (например src\pages\paint\components\Canvas\Canvas.tsx) Значения setTimeout также в константу с понятным названием (например src\core\components\Modal\Modal.tsx) Ссылки тоже в константы (например src\pages\Home\HomePage.tsx)

2) Если используешь styled-components, то в className нет необходимости (много где вижу), используй стайлдед вместо этого (src\pages\paint\PaintPage.tsx)

3) Избегай использования inline функций и стилей (встречается много где)

4) Не совсем понял зачем id на элементе?

5) Управление модалками в принципе в верном направлении мыслишь, но лучше это систематизировать, вот неплохо пример как можно сделать: https://andremonteiro.pt/react-redux-modal/

6) Utility для рисования фигур думаю можно вынести в отдельный файл хелпер в той же папки, функции будут принимать ctx и нужные аргументы (src\pages\paint\components\Canvas\Canvas.tsx)

7) Используй строковые шаблоны всегда вместо конкатенации (src\pages\Home\HomePage.tsx)

8) В списке плозая практика указывать в качестве ключа индекс, у теюя же там есть id, на крайний случай ссылка тоже как уникальный идентификатор сгодиться (src\pages\Home\HomePage.tsx)

9) Логика handleSubmit должна быть в родительском компоненту формы, в ообщемто в этом и есть смысл разделения наотдельный компонент этой формы (src\pages\Auth\SignUp\components\SignUpForm\SignUpForm.tsx) В это случае форма отвечает только за валидации и отдачу данных, а родительский компонент за логику обработки эти данных (апи запрос, диспатч экшена и тд)

10) В саге вместо takeEvery лучше использовать takeLatest, чаще всего так и есть, причина в том что нам нужен только последний результат (акутальный), а экшен может эмититься нескольк раз в разных местах где нужны те же данные, в таком случае чтобы не делать лишних запросов лучше юзай takeLatest, например нажатие кнопки если просиходит несколько раз, но нам нужен только послежний. Прочитай про отличия этих операторов ещё раз. (src\core\sagas\auth.ts)

11) Помнишь правило один интерфейс = один файл (бываеют исключения, но чаще всего так и есть). Интерфейс тсейта должен быть у его редьюсера (src\core\interfaces\states.ts) В примере проекта который я скидывал это очень хорошо сделано и организовано. Тоже самое с AuctionTypes каждый из которых можно разместить в соотвествующем файле с экшенами (src\core\constants\actionTypes.ts)

[Rabtsevich-Ivan]

У меня есть несколько вопросов. Буду указывать пункты твоего ревью и вопросы к ним 1)Сделал всё, кроме: <Значения setTimeout также в константу с понятным названием (например src\core\components\Modal\Modal.tsx)> Не совсем понимаю, что ты имеешь в виду

Update: Есть предположение. Гипотетически у нас в приложении будет несколько совершенно разных модалок на разных страницах в разных компонентах. Ты хочешь, чтобы за рендер каждого из них отвечал один и тот же функционал, только с разными входными данными. И эти данные, описывающие нашу конкретную модалку из нескольких и нужно вынести в отдельную константу. Правильно я тебя понял?

Update 2: Скорее всего неправильно. Т.к. у меня setTimeout просто закрывает модалку через 1.5 секунды и все, а тема выше будет не в setTimeout, а в (src\pages\Paint\components\PaintControl\PaintControl.tsx) - handleModal

[Rabtsevich-Ivan]

2)Ты прав. Я удалил в паре моментов classNames, которые не использовал. Но есть один момент, в котором я не уверен, что делать. Вопрос. Правильно ли я понимаю, что мне нужно Link обернуть в styled component? Update 0: Нет, неправильно. Этот момент я уже решил - переходи к следующему export const NavRouterLink = styled(Link)' css styles... '

Update: Я подключал библиотеку fontAwesome. Здесь тоже осталось className. Стоит ли мне с этим что-то сделать или оставлять так? (осталось только это - все остальные заменил на styled Components)

Update 2: Нашёл вот эту библиотеку = styled Icons. Стоит ли мне заменять fontAwesome иконки с className на это? https://styled-icons.js.org/?s=circle

[Rabtsevich-Ivan]

3) Inline стили я не использую, а по поводу inline функций - ты прав. Почитал статьи на тему оптимизации React. В статьях датируемых годами 2017=2018 пишут, что не стоит запариваться с inline function, так как их они практически не влияют на performance, а оптимизируя их и вынося эти колбеки в компонент - мы замедляем инициализацию компонента. Но в новых статьях - 2020 по 2021 уже пишут, что их нужно избегать. И тут же у меня вопрос. Avoid не значит вообще не использовать. Я у себя везде подправлю, но чисто на будущее, есть ли случаи, где inline функции оправданы? Или мне их никогда не использовать и все колбеки выносить в компонент (как я это делал с submitHandle?)

Update: Вот, например, здесь нужно мне оставлять inline function или выносить её в handler? (Везде, кроме этого случая, вынес функцию в компонент)

[Rabtsevich-Ivan]

4) <Не совсем понял зачем id на элементе?> Если ты про контейнер для картинок в (src\pages\Home\HomePage.tsx) то я не знаю. Чисто на всякий случай добавил. Но так как их(id) нигде не использую - уже убрал

[Rabtsevich-Ivan]

5)Под систематизацией, ты имеешь ввиду, чтобы у меня модалка была не только в (src\pages\Paint\components\PaintControl\PaintControl.tsx), где я её расположил, чтобы передать в неё пропсы, а в файле app.tsx и передать пропсы через redux action как в конце статьи?

7, 8, 11 уже сделал, а во всем остальном - вопросов нет. В идеале, сделаю всё сегодня

[Rabtsevich-Ivan]

10)Действительно. Прочитал, что takeLatest автоматически отменяет предыдущие запросы и принимает только последний. На практике проверил на отображении ошибки, когда пробую залогиниться с неправильными данными. takeEvery выдает ошибку сразу же (с первого запроса, даже если много кликаю). А takeLatest тормозит отображение ошибки, даже, если много нажимаю. С другой стороны - так как ошибка у меня отображается от redux(статус ошибки в boolean значении хранится в state) - отображение ошибки занимает время. Грубо говоря, использую takeLatest на login (там try {} catch( здесь put(error)) ) - проблема теперь в том, что отображение ошибки визуально занимает слишком много времени. Это нужно показывать на созвоне, но если словами - если пользователь часто и без конца кликает на логин с неправильными данными, ошибка о том, что данные неправильные, отобразится только тогда, когда он прекратит быстро кликать (запрос занимает время и отменяется еще до того, как завершился)

[bitvalser]

1) Немного неправильно. У тебя есть такой код setTimeout(() => { dispatch(closeModal()); }, 1500); Это вещь будет непонятно сразу другим людям в команде, что она значит и для чего, поэтому выносим это значения за компонент и называем понятным названием и тогда не нужно будет искать это значение и легко менять его:

const MODAL_CLOSE_DELAY = 1500; 
 {...}
setTimeout(() => {
  dispatch(closeModal());
}, MODAL_CLOSE_DELAY);

А вообще я пересмотрел логику и мне не очень нравиться обработка такая, зачем задержка? А если у меня инет мделенный и сохраняться будет 5 секунд? Модалку по хорошему закрывать сразу после того как сохранилась картинка, например в саге после успешного сохранения диспатчить экшен на зарытие.

2 - В FontAwesome есть отдельный пакет для реакта https://fontawesome.com/v5.15/how-to-use/on-the-web/using-with/react

3 - Это не особо будет на перфоманс влиять, но таким образом читать код проще, когда вся логика как можно больше разделена от представления, чтобы всё что выполняется в компоненте можно увидеть до return, а дальше только представление. Пример который ты привёл можно так и оставить, так как ты рендеришь комопнент таким образом то я думаю в таких случаях можем делать исклюения, учитывая что мы много где используем инлайн функцию при рендере наприме списком.

5 - Я просто хочу чтобы логика управления модалками была систематизирована, чтобы можно было реиспользовать её, чтобы не дублировать много кода, чтобы за их управления отвечал определенный механизм. То есть да, нужно сделать как на том сайте (ссылку я скидывал), это один из вариантов и он неплохой. Чтобы та модалка которую ты сделал была через тот механизм который описан.

10 - Да и пусть не показывается пока кликает, это не проблема, то что запрос отменяется это запланированное событие так и нужно.

[Rabtsevich-Ivan]

Момент по поводу модалки. Когда будешь проверять, как я сделал, знай, что я знаю, что можно сделать так: https://ru.reactjs.org/docs/composition-vs-inheritance.html#specialization Я просто не знал как более правильно, а потому выбрал более интуитивный для меня способ.