Show your code Saturday: код-рев’ю #4

💡 Усі статті, обговорення, новини про Front-end — в одному місці. Приєднуйтесь до Front-end спільноти!

Коментар редакції. Нещодавно ми оголосили про початок Show your code Saturday — це інтерактивна рубрика, у якій кожен охочий може поділитися власним проєктом, написавши про нього в коментарях. А ми натомість щосуботи обираємо найцікавіший і розповідатимемо детальніше.

Сьогодні ми хочемо поділитися з вами роботою Юрія Артеменка.

Наш активний учасник спільноти Максим Рудний зробив код-рев’ю проєкту! Тож далі передаємо слово Максиму 👇

Дисклеймер: я не є автором коду і не знаю причин реалізації будь-якої частини коду таким чином, як це є. Стиль написання коду без ознайомлення з Code Style та Code Convention проєкту не можу критикувати, може це так було задумано.

Сьогодні на огляді у нас цікавий проєкт від Юрія — сервіс, де можна вибирати та відмічати переглянути епізоди Сімпсонів. Дякую автору, що поділився своїм кодом.

Що не так з цим проєктом

Коли я глянув уважніше код, в мене виникло декілька питань. Перш за все, цікавить вибір стеку та технологій. Іншим важливим питанням є авторизація на сайті. Для навчального проєкту і щоб «погратися» з технологіями та рішеннями — це ок.

Для робочого застосунку — занадто складно і недоцільно, до того ж небезпечно. Я поламав сайт за кілька хвилин. Про це більше у відео.

Чистота коду

Глянемо на код, де вибираємо backgroundColor. Хто може сказати, як підтримувати або змінювати цю умову?

backgroundColor:
 !state?.isWatched && !state?.isForLater
   ? "#354051"
   : state?.isForLater
   ? "#00a2ff"
   : episode.episode % 2 === 0
   ? "#ebb926"
   : "#fbcf48",

В лінтерах зазвичай є правило, що забороняє використовувати тернарний оператор всередині тернарного оператора, а тут він використаний тричі (може і більше — дуже важко читати такий код). Визначення кольору потрібно винести в окрему змінну або функцію і спростити. В ідеалі ще й юніт-тест написати.

Ще один приклад занадто складної логіки:

const newState = {
 ...state,
 isLiked:
   state && "isLiked" in state && state.isLiked === false
     ? undefined
     : false,
};

Ось так визначати стейт не потрібно. Тут допущена відразу декілька помилок. Про це у відео детальніше.

Коментарі

Коментарі брешуть! Код повинен говорити сам за себе. Видаляйте непотрібні коментарі з описом того що робить функція. Наприклад:

// when user is registered propose login and create alert
useEffect(() => {
 if (isRegisterSuccess) {
   setType("Login");

Можна замість анонімної функції в ефекті вказати іменовану, які й пояснять, що тут робиться.

useEffect(propeseLoginAndCreateAlert() {
 if (isRegisterSuccess) {
   setType("Login");

Але і це рішення не найкраще. Порушений принцип Single Responsibility — функція повинна робити щось одне.

Безпека

Самописна авторизація яка створює додаткові проблеми. З урли взяли токен

const tokensParams = req.nextUrl.searchParams.get("tokens");

Поклали його в куки

response.cookies.set("tokens", tokensParams, {
 maxAge: 60 * 60 * 24 * 7,
});

І потім пробуємо розпарсити

tokens = JSON.parse(req.cookies.get("tokens")?.value as string);

Як мінімум, JSON.parse треба розмістити в try…catch(e), щоб уберегтися від помилок, коли там буде не JSON або він буде не валідний, інакше можна зламати сайт, як я це зробив.

Висновок

Як навчальний — це чудовий проєкт. Використані популярні технології, які часто вимагаються в компаніях — NextJS, Redux Toolkit, Material UI. Написана авторизація самотужки.

Як продакшин чи комерційний проєкт, на основі того що зроблено, стек вибраний не найкращий. Занадто складно і багато додаткової роботи зроблено.