Code-review тестового задания junior react разработчиков

Что это такое?

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

Общие проблемы

• Плохое Readme;
• Остаются eslint предупреждения, лишние console.log (redux-логгер не в счет);
• Иконка Web не вынесен вперед (невнимательное чтение задачи);
• Иконка Web выносится вперед в компоненте (а лучше бы в редьюсере, или экшене);
• Пароль не очищается, если запрос вернулся с ошибкой;
• Submit кнопка в форме логина доступна, если поля пустые (или одно из полей);
• Submit кнопка в форме логина не поддерживает нажатие Enter;
• Нет деления на компоненты/контейнеры (не относится к тем, кто делил по другим подходам);
• URL-адрес для запросов на сервер полностью передается (нет оформления повторяющейся части строки в константу);
• Ошибка/уведомление "неправильное имя пользователя/пароль" — не очищается;
• Ошибка "неправильное имя пользователя/пароль" — выводится константой с сервера;
• Текст ошибки "захардкожен" в коде. Нет обращения в словарик по константе с сервера;
• Не удален "старый код", то есть такой код, который нигде не используется;
• Нет блока catch у промисов, нет обработки ошибок, если сервер отвечает не ok;
• Компоненты размещены в node_modules;
• Отсутствуют или недостаточно подробно описаны Prop Types.
• Экшены и редьюсеры в куче в одном файле (или в одном все экшены, в другом все редьюсеры). Нет деления на "модули", то есть каждой сущности — свои экшены и свои редьюсеры;

Все решения с отметками по времени

Здесь указаны только ошибки, и не отмечены хорошие моменты, которых великое множество, поэтому крайне рекомендую посмотреть все подряд тем, кто считает себя "джуном" в разработке на react.

---

6m00s — Артур Донковцев Commit

7m40s — опечатка в названии функции

8m07s — асинхронные запросы не вынесены в экшены

---

9m30s — Павел Пимкин Commit

10m07s — все экшены в одном файле. Нет деления на модули.

10m25s — вынос иконки (перебор данных) сделан в компоненте. Лучше в редьюсере или экшене.

---

11m42s Сергей ZackFox Commit

12m28s — "прикольные" надписи. Лучше делать "нейтрально", чтобы затем подобные задания можно было сразу посылать работодателю.

13m05s — лишний экшен, указывающий что "загрузка" завершилась. То есть вместо трех экшенов: REQUEST / SUCCESS / STOP, можно уложиться в два: REQUEST / SUCCESS.

---

16m16s — Дмитрий Петров Commit

18m16s — использование var

18m34s — не вынесена часть URL адреса в константу

---

21m15s — Ефим Хлебный Commit

21m17s — плохое сообщение в коммите

22m15s — одинаковые названия экшенов.

---

24m16s — Кацура Владислав Commit

25m17s — (не ошибка) — данные приготовлены в редьюсере

27m38s — использование e.target, лучше e.currentTarget

28m20s — ==, а надо бы ===

28m33s — использование componentWillUnmount

29m00s (не ошибка) — рассуждение про "до серверную валидацию".

30m05s — код не отфморатирован (на любителя)

---

30m33s — Максим Сафин Commit

31m35s — использование "не универсального" обработчика, там где это уместно.

---

32m02s — Сергей Regies Linkas Commit

33m42s — нет экшена для прелоадера

34m30s — порядок методов в компоненте. (eslint плагин)

35m30s — не существующий PropTypes

---

35m57s — Кононов Виталий Commit

---

38m02s — Ренат Рысаев Commit

39m45s — не делаем то, что не интересно

---

40m31s — Евгений Санжиев Commit

41m20s (не ошибка) — словарик для работы с ошибками

---

42m46s — Виталий Набережный Commit

42m54s — не убраны тестовые данные

---

44m50s — Вениамин Трепачко Commit

Ачивка: очень классный дизайн.

47m42s — redux версия не полноценная.

---

47m57s — Ingvarr6 (Игорь) Commit

48m21s — нет 404 роута

---

51m20s — Екатерина Н Commit

51m30s — не очищается ошибка

---

54m48s — Роман Палесика Commit

55m30s — не хватает экшенов на загрузку/ошибку

56m49s — использование side-effects в редьюсере

58m10s — (не ошибка) вынос иконки web с помощью css (sick!)

---

58m53s — Умяр Юсупов Commit

59m15s — использование callback'a в setState, что приводит к лишней перерисовке. Лучше валидировать прямо в рендере.

61m01s — неуместное использование else if

---

62m13s — dsfcv d (boortcore) Commit

---

63m15s Константин Липский Commit

65m11s — в экшен передается URL целиком, лучше просто id передавать в данном варианте.

---

67m14s — Ikaow Ikaow Commit

67m50s — сложное условие в shouldComponentUpdate, можно проще (сразу проверить на props.data и все)

69m32s — e.preventDefault не первый в обработчике

---

70m01s — Ali Gasymov Commit

---

71m50s — Ахметанов Альберт Commit

72m20s — компоненты в node_modules

73m15s — дублирование обращений к переменным

---

74m04s — Женя Белый Commit

76m04s — privateRoute не вынесен в отдельный компонент

76m33s — сложный код для перемещения иконки web

76m56s — избыточное свойство loaded

---

77m35s — Аладьин Александр Commit

80m33s — ошибка не вынесена в словарик

---

81m19s — Misha Mihail Commit

81m43s — избыточное использование withRouter

---

83m04s — Dmitrii Shapovalenko Commit

---

84m00s — Даниил Commit

84m58s — избыточное деление экшенов

85m55s — ошибка в названии lifecycle-метода

---

86m58s — Порошин Роман Commit

87m15s — семантически неверное использование тэга article

90m46s — лишний вызов метода массива

---

91m10s — Артем Бочков Commit

Автор: maxfarseer

Источник