- PVSM.RU - https://www.pvsm.ru -

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

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

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

выбрал реакт

Видео

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

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

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

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

6m00s [2] — Артур Донковцев Commit [3]

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

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

9m30s [6] — Павел Пимкин Commit [7]

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

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

11m42s [10] Сергей ZackFox Commit [11]

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

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

16m16s [14] — Дмитрий Петров Commit [15]

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

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

21m15s [18] — Ефим Хлебный Commit [19]

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

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

24m16s [22] — Кацура Владислав Commit [23]

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

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

28m20s [26] — ==, а надо бы ===

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

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

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

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

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

32m02s [33] — Сергей Regies Linkas Commit [34]

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

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

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

35m57s [39] — Кононов Виталий Commit [40]

38m02s [41] — Ренат Рысаев Commit [42]

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

40m31s [44] — Евгений Санжиев Commit [45]

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

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

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

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

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

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

47m57s [53] — Ingvarr6 (Игорь) Commit [54]

48m21s [55] — нет 404 роута

51m20s [56] — Екатерина Н Commit [57]

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

54m48s [59] — Роман Палесика Commit [60]

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

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

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

58m53s [64] — Умяр Юсупов Commit [65]

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

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

62m13s [68] — dsfcv d (boortcore) Commit [69]

63m15s [70] Константин Липский Commit [71]

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

67m14s [73] — Ikaow Ikaow Commit [74]

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

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

70m01s [77] — Ali Gasymov Commit [78]

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

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

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

74m04s [83] — Женя Белый Commit [84]

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

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

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

77m35s [88] — Аладьин Александр Commit [89]

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

81m19s [91] — Misha Mihail Commit [92]

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

83m04s [94] — Dmitrii Shapovalenko Commit [95]

84m00s [96] — Даниил Commit [97]

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

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

86m58s [100] — Порошин Роман Commit [101]

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

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

91m10s [104] — Артем Бочков Commit [105]

Автор: maxfarseer

Источник [106]

Сайт-источник PVSM.RU: https://www.pvsm.ru

Путь до страницы источника: https://www.pvsm.ru/javascript/281865

Ссылки в тексте:

[1] второго тестового задания: https://github.com/maxfarseer/tz-webinars/tree/tz-2-react-redux-router-async

[2] 6m00s: https://youtu.be/EWaNgM4mv-A?t=6m00s

[3] Commit: https://github.com/DonkovtsevArthur/testTask/tree/4dbebf493427450e310591ad72cf51150c7c57c6

[4] 7m40s: https://youtu.be/EWaNgM4mv-A?t=7m40s

[5] 8m07s: https://youtu.be/EWaNgM4mv-A?t=8m07s

[6] 9m30s: https://youtu.be/EWaNgM4mv-A?t=9m30s

[7] Commit: https://github.com/paxarpp/react_training/tree/26d8d60b424742415be38deb76afc4d682bc6cfc

[8] 10m07s: https://youtu.be/EWaNgM4mv-A?t=10m07s

[9] 10m25s: https://youtu.be/EWaNgM4mv-A?t=10m25s

[10] 11m42s: https://youtu.be/EWaNgM4mv-A?t=11m42s

[11] Commit: https://github.com/ZackFox/frontend-react-jt2/tree/f92b48d988f250ef3feb16a4270f8437c8f1f3ee

[12] 12m28s: https://youtu.be/EWaNgM4mv-A?t=12m28s

[13] 13m05s: https://youtu.be/EWaNgM4mv-A?t=13m05s

[14] 16m16s: https://youtu.be/EWaNgM4mv-A?t=16m16s

[15] Commit: https://github.com/Junglecrew/Forge-2/tree/765d97ddee2437bd2dae2480dc2a4724f89684fa

[16] 18m16s: https://youtu.be/EWaNgM4mv-A?t=18m16s

[17] 18m34s: https://youtu.be/EWaNgM4mv-A?t=18m34s

[18] 21m15s: https://youtu.be/EWaNgM4mv-A?t=21m15s

[19] Commit: https://github.com/Chicloon/react-test/tree/de178ab0988a02d8a800b65516eb0c4fd2d2ef7b

[20] 21m17s: https://youtu.be/EWaNgM4mv-A?t=21m17s

[21] 22m15s: https://youtu.be/EWaNgM4mv-A?t=22m15s

[22] 24m16s: https://youtu.be/EWaNgM4mv-A?t=24m16s

[23] Commit: https://github.com/missilev/Maxpfrontend-Test-Task-2/tree/cbc20fb02f54230c81c8bf46109684f4f8c05d00

[24] 25m17s: https://youtu.be/EWaNgM4mv-A?t=25m17s

[25] 27m38s: https://youtu.be/EWaNgM4mv-A?t=27m38s

[26] 28m20s: https://youtu.be/EWaNgM4mv-A?t=28m20s

[27] 28m33s: https://youtu.be/EWaNgM4mv-A?t=28m33s

[28] 29m00s: https://youtu.be/EWaNgM4mv-A?t=29m00s

[29] 30m05s: https://youtu.be/EWaNgM4mv-A?t=30m05s

[30] 30m33s: https://youtu.be/EWaNgM4mv-A?t=30m33s

[31] Commit: https://github.com/A4ron5/apptest/tree/71d57ea574b5c1197d4ea45887523a1cab4440c9

[32] 31m35s: https://youtu.be/EWaNgM4mv-A?t=31m35s

[33] 32m02s: https://youtu.be/EWaNgM4mv-A?t=32m02s

[34] Commit: https://github.com/sinneren/tz-webinars/tree/04c84bdac0466e5b85b9607f554cc5e63a42dc41

[35] 33m42s: https://youtu.be/EWaNgM4mv-A?t=33m42s

[36] 34m30s: https://youtu.be/EWaNgM4mv-A?t=34m30s

[37] eslint плагин: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/sort-comp.md

[38] 35m30s: https://youtu.be/EWaNgM4mv-A?t=35m30s

[39] 35m57s: https://youtu.be/EWaNgM4mv-A?t=35m57s

[40] Commit: https://github.com/vital2207/ReactTestProject/tree/d0d8308eb21af0392c38965598f843e2dbfbc1e5

[41] 38m02s: https://youtu.be/EWaNgM4mv-A?t=38m02s

[42] Commit: https://github.com/RenatRysaev/knowledge-test/tree/c4a31d57689f12637820b9868e57369ab4c15e13

[43] 39m45s: https://youtu.be/EWaNgM4mv-A?t=39m45s

[44] 40m31s: https://youtu.be/EWaNgM4mv-A?t=40m31s

[45] Commit: https://github.com/seaone/react-test/tree/f72e490a9d0a12f99c55ae3fb35b54757b835cfa

[46] 41m20s: https://youtu.be/EWaNgM4mv-A?t=41m20s

[47] 42m46s: https://youtu.be/EWaNgM4mv-A?t=42m46s

[48] Commit: https://github.com/vvnab/maxpfrontend/tree/9e7b689ff5cf8f546ef73fac7fe6a5fc805a3a65

[49] 42m54s: https://youtu.be/EWaNgM4mv-A?t=42m54s

[50] 44m50s: https://youtu.be/EWaNgM4mv-A?t=44m50s

[51] Commit: https://github.com/miniven/news-tutorial/tree/34b82a5a505b45535f7ec1f0f476cb8d70fca298

[52] 47m42s: https://youtu.be/EWaNgM4mv-A?t=47m42s

[53] 47m57s: https://youtu.be/EWaNgM4mv-A?t=47m57s

[54] Commit: https://github.com/ingvarr6/Test_2/tree/ebe89e4739a14ef1987740082cb5c0a455656c39

[55] 48m21s: https://youtu.be/EWaNgM4mv-A?t=48m21s

[56] 51m20s: https://youtu.be/EWaNgM4mv-A?t=51m20s

[57] Commit: https://github.com/NKaty/React_test_task2_maxpfrontend/tree/3df7c3f033cc0b659ba15e4e3b4ca5735d8b837d

[58] 51m30s: https://youtu.be/EWaNgM4mv-A?t=51m30s

[59] 54m48s: https://youtu.be/EWaNgM4mv-A?t=54m48s

[60] Commit: https://github.com/shesperfect/paca/tree/2973b398b5566c4d4a4a3ae0ef6e2a9b1605afbb

[61] 55m30s: https://youtu.be/EWaNgM4mv-A?t=55m30s

[62] 56m49s: https://youtu.be/EWaNgM4mv-A?t=56m49s

[63] 58m10s: https://youtu.be/EWaNgM4mv-A?t=58m10s

[64] 58m53s: https://youtu.be/EWaNgM4mv-A?t=58m53s

[65] Commit: https://github.com/umyar/react/tree/78c8887e0479c565568101aeffb3ef265efd3539

[66] 59m15s: https://youtu.be/EWaNgM4mv-A?t=59m15s

[67] 61m01s: https://youtu.be/EWaNgM4mv-A?t=61m01s

[68] 62m13s: https://youtu.be/EWaNgM4mv-A?t=62m13s

[69] Commit: https://github.com/Boortcore/tz-webinars/tree/c8af4f7b247bd9ca555641ee8e0d7de1b1907425

[70] 63m15s: https://youtu.be/EWaNgM4mv-A?t=63m15s

[71] Commit: https://github.com/KonstantinLypskyi/TZ_2/tree/4c283d96e2779a4688155faa2a3d84a896571892

[72] 65m11s: https://youtu.be/EWaNgM4mv-A?t=65m11s

[73] 67m14s: https://youtu.be/EWaNgM4mv-A?t=67m14s

[74] Commit: https://github.com/ikaow/front/tree/aa27bef13c475de0ecebcba17f0e053fff302617

[75] 67m50s: https://youtu.be/EWaNgM4mv-A?t=67m50s

[76] 69m32s: https://youtu.be/EWaNgM4mv-A?t=69m32s

[77] 70m01s: https://youtu.be/EWaNgM4mv-A?t=70m01s

[78] Commit: https://github.com/alik0211/socgram/tree/109f8cd7e976a8f9e442aa68c0027cfaaeea7df2

[79] 71m50s: https://youtu.be/EWaNgM4mv-A?t=71m50s

[80] Commit: https://github.com/adacs897642/react_test_pr/tree/cb6ace56b9b4f5d8650dc395255fc2b858ed0558

[81] 72m20s: https://youtu.be/EWaNgM4mv-A?t=72m20s

[82] 73m15s: https://youtu.be/EWaNgM4mv-A?t=73m15s

[83] 74m04s: https://youtu.be/EWaNgM4mv-A?t=74m04s

[84] Commit: https://github.com/white1984j/task-from-maxpfrontend/tree/e07405396f6188fb679250dfbb9e28863d5027c7

[85] 76m04s: https://youtu.be/EWaNgM4mv-A?t=76m04s

[86] 76m33s: https://youtu.be/EWaNgM4mv-A?t=76m33s

[87] 76m56s: https://youtu.be/EWaNgM4mv-A?t=76m56s

[88] 77m35s: https://youtu.be/EWaNgM4mv-A?t=77m35s

[89] Commit: https://github.com/sanchos86/test-2/tree/161e01e0c440763476f4f8bd207c2a620f94d1f8

[90] 80m33s: https://youtu.be/EWaNgM4mv-A?t=80m33s

[91] 81m19s: https://youtu.be/EWaNgM4mv-A?t=81m19s

[92] Commit: https://github.com/MichaelSedov/frontend-react-test2/tree/90d75e3a36e6bddbc2afb6723fa8a37682d1e207

[93] 81m43s: https://youtu.be/EWaNgM4mv-A?t=81m43s

[94] 83m04s: https://youtu.be/EWaNgM4mv-A?t=83m04s

[95] Commit: https://github.com/ShapovalenkoD/react-authorization-test/tree/943eb5e5f7461d2ecfa7d60db3dbacaa373c2f0e

[96] 84m00s: https://youtu.be/EWaNgM4mv-A?t=84m00s

[97] Commit: https://github.com/skyskysky11/tz2/tree/67ae81378a0bcd48e89d5923a0534c86e24ac19e

[98] 84m58s: https://youtu.be/EWaNgM4mv-A?t=84m58s

[99] 85m55s: https://youtu.be/EWaNgM4mv-A?t=85m55s

[100] 86m58s: https://youtu.be/EWaNgM4mv-A?t=86m58s

[101] Commit: https://github.com/mkitez/react-redux-test-app/tree/4e6c62cf6f464ba2b679cb38e43d5c6fd64adf66

[102] 87m15s: https://youtu.be/EWaNgM4mv-A?t=87m15s

[103] 90m46s: https://youtu.be/EWaNgM4mv-A?t=90m46s

[104] 91m10s: https://youtu.be/EWaNgM4mv-A?t=91m10s

[105] Commit: https://github.com/artbocha/react-redux-test-task-2/tree/146f0d4f1972aaa325318fb630d432b384c61540

[106] Источник: https://habr.com/post/413135/?utm_campaign=413135