В данной заметке отражены плюсы и минусы работ начинающих react-разработчиков. Вы можете подсмотреть как с одной и той же задачей можно справиться используя различные подходы.
Разбор данной задачи в решениях на TypeScript будет позже, следите за новостями в telegram канале
Общие пожелания:
- Создавать компоненты без state с помощью функций (а не классов)
- Логику проверки имени и пароля пользователя нужно выносить из компонента login или подобного
- Кнопка для логина, должна быть заблокирована, пока форма не заполнена
- Форма логина должна быть сверстана с помощью тэга form + обработка onSubmit, чтобы Enter работал
- Выкладывайте demo
- Пишите подробное README
Robert Zagidulin
Минусы
Логику проверки имени пользователя/пароля нужно вынести из компонента (в action creator).
index в качестве ключа — опасно. Так как у нас учебный пример, можно было добавить новости id и использовать его.
Жестко вбитые данные на странице профиля.
В редьюсерах нет информации о том, загружаются ли данные в данный момент. Как и нет action creators в целом. Нужно было сэмулировать работу через цикл: случился экшен, приехали даные, отрисовался компонент.
Нейтрально:
Использован подход с рендером через функцию, можно сделать PrivateRoute компонент, в итоге может оказаться удобнее и меньше символов.
Плюсы:
Для шаблона использован подход с отрисовкой children
Есть UI компоненты.
Любопытно:
Отрисовка полей формы через универсальную функцию
Бомбин Сергей
Минусы:
Нет разделения на контейнеры и компоненты.
Плюсы:
Использованы хуки, например в LoginPage
Использован сервис «тестовых новостей» — jsonplaceholder
Konstantin Filipenkov (Kos)
Минусы:
Повторюсь, про незаблокированную кнопку для Login формы, если данных нет (т.е. форма не валидная)
Есть pages/News
(считай контейнер), но connect почему-то происходит в компоненте
Слишком нагло Автор, скорее всего хотел не запрашивать новости, если они уже запрошены, но к очередному заходу на новости — данные для этой страницы уже могли поменяться (если у нас реальный бэкэнд). Так же, можно почитать данный кусочек документации — What can I do if my effect dependencies change too often?
Нейтрально
Использован Fragment
, можно просто <>
Универсальная отрисовка роутов (скорее плюс, чем нейтрально).
Плюсы:
Автофикс линтером вынесен в отдельный скрипт
Используется precommit
Использование html5 тэгов, например, main
Логика получения информации об авторизации вынесена из компонента. Используются cookies. Логика проверки имени пользователя/пароля так же вынесена.
В редьюсере присутствуют нужные поля (можно четко определить статус загрузки, ошибку, данные)
Axios настроен в API
Кирилл Прага
Минусы:
warning в консоли
Sign in кнопка на форме не работает
Не хватает информации в редьюсере/экшенах для асинхронной загрузки новостей.
Логику запроса новостей следует вынести из компонента
Если свойство обязательно, не забываем проставлять isRequired
Константин Сидоров
Yarn!
Нейтрально
Раз уж решились взяться за JSDoc формат комментариев, то хотелось бы видеть в них больше информации.
Используются styled-components (скорее плюс)
Используется formik
Чтобы избавиться от длинных ./../../../
в импортах, можно использовать alias.
Минусы
Зачем delete/get/set cookie разнесены в 3 директории?
Придираюсь, так как автор заморочился — в withLogin
значения для регулярок, можно было бы вынести в константы, например VALID_USERNAME
и VALID_PASSWORD
, чтобы это выглядело более читаемо для других разработчиков.
В случае с проблемой загрузки новостей, вбрасывается ошибка (это плюс!), но она нигде не обрабатывается? Это же касается компонента новостей, если произошла ошибка, можно было бы показать кнопку: «запросить снова». Компонент новостей (и редьюсер) не готовы, чтобы отобразить ошибку.
Хорошая работа, почему без propTypes? Про тесты не спрашиваю
Плюсы
.eslintrc пожирнее
Использован reach-router
Для структуры проекта используется Atomic Design
Используется стороннее API для запроса новостей
Работа заслуживает внимания.
Баходир Кадыров
Минусы
Все файлы в одной куче, нет деления на умные/тупые компоненты.
Нет возможности асинхронной загрузки новостей
Нейтрально
Почему hashRouter?
Использование material-ui
Пожелания
Организовать структуру файлов/директорий. Добавить propTypes, добавить загрузку новостей в асихнронном варианте.
Roman (iromjkeecorporation)
На удивление многие используют jsonplaceholder
Минусы
У страницы новостей нет прелоадера
Не вынесен отдельный компонент NewsItem (для отрисовки одной новости).
Плюсы
Проверка логина/пароля (checkCredentials) вынесена
Fyodor P
Используется semantic-ui
Минусы
Варнинги в консоли.
Дизайн жестковат.
Подписка в компоненте новостей на весь state целиком
Название переменной, а где initialState2?
Не самые удачные названия экшенов, например TAKE_DATA ? Мы хотим получить данные, ок, но какие? Куда?
Мертвый код? Ниже нигде не используется this.textInput
Используется доступ к переменным через this.state, хотя выше они вынесены в const (невнимательность!)
noombasa
Используется redux-saga
Минусы
Текст сообщения в коммите
Нейтрально
Единорог
Плюсы
Но используются функциональные компоненты (плюс)
Присутствуют хэлперы (проверка имени пользователя, работа с LocalStorage)
Alexander (SonikDropout)
Используется react-bootstrap
Так как у многих этого сделано не было, хочу отметить здесь классический редьюсер с состоянием загрузки/ошибки/пришедших данных. Сюда же — классический асинхронный запрос за данными в action creator’e
Используется setState hook (плюс), но здесь стоит думать о том, что данные про юзера (правильный/неправильный логин пароль) придут к нам с сервера. Поэтому такая проверка здесь не прокатит.
Используется useEffect хук для запроса новостей.
Temir Adzhiahetov
Минусы
Новости предполагалось прокинуть через redux, но тут речь про мертвую переменную в state (isLoading)
При первой загрузке приложения, в шапке сразу же кнопка «Выйти» (хотя никто еще не заходил) и следовательно — страница профиля доступна без ввода данных.
Плохое именование переменных в LocalStorage
Const и Var в перемешку.
Для 3х и более if используем switch case!
Забытые console.log
Severin Aimo
Минусы
По сути, вся работа выполнена в одном коммите. Лучше, чтобы было несколько последовательных коммитов, так работодатель может проверить ваш навык работы с гитом.
Где «прогон через eslint» / разбивка на строки?
Нет загрузки новостей.
По названиям экшенов не очень понятно, что будет происходить.
Логику лучше отделять от компонентов, в нашем случае не только проверка пользователя, но и установка LS (лучше делать в едином месте, например в экшене)
Плюсы
PropTypes на месте, используется styled-components.
Толошный Д.А.
Пхп разработчик, перидоически пытаюсь ворватся в реакт-фронтенд, но что-то тяжко.
Минусы
Не указаны все зависимости (в частности, react-router-dom)
Предупреждения в консоли
В App.js больше 1го компонента, которые можно вынести.
Длинные строки (eslint?)
PropTypes доделать!
Забытые console.log
Пожелания
Внимательность, по redux — перечитать учебник и документацию.
Necelentano
Демо на codesandbox
Минусы
Классический пример контейнера новостей в котором не хватает прелоадера (минус), при том, что информация в редьюсере есть!
Нет необходимости в pages и containers. По сути, pages может быть контейнером.
Аккуратная работа, все нормально, но где, черт подери, propTypes?
Плюсы
Нет вопросов к реализации стора (редьюсеры ОК, экшены ОК). В константах все в порядке, но можно бы добавить namespace через слэш, например вместо LOGIN_REQUEST
, login/REQUEST
(это вкусовщина).
Проверка пользователя и логаут вынесены отдельно