Month10/step1 by ulbee · Pull Request #8 · ulbee/react-burger

ulbee · 2023-04-02T22:59:21Z

No description provided.

…хранилища даннях пользователя и ингридиентов

ArslanMustafin

Code Review

Здравствуйте!

Отличная работа.

Вы молодец!

Сборка и запуск проекта выполняются без ошибок;
Проект теперь типизирован;

Но есть несколько замечаний, которые нужно исправить:

Отметил замечания в коде;

Сделает код лучше:

Стоит обращать внимание на предупреждения в терминале, в котором запущен проект. Это хорошие рекомендации, которых следовало бы придерживаться. В противном случае, лучше отключить линтер у строки (куска кода) eslint-disable. Так же добавить комментарий (TODO), объясняющий причину блокировки линтера в данном месте. Суть в том, что мы стараемся писать более чистый код, а если видим, что порой выскакивают предупреждения того же линтера, то оставляем комментарий, объясняющий причину. В коммерческой разработке, когда есть целая команда разработчиков, работающих над одним проектом, эта рекомендация послужит правилом хорошего тона. Неплохая статься по поводу локального отключения линтера: https://learn.coderslang.com/ru/0023-eslint-disable-for-specific-lines-files-and-folders

Подробно эти и более мелкие замечания, отмечены в коде.

После исправления всех замечаний, работа будет принята)

ArslanMustafin · 2023-04-03T05:39:59Z

src/services/actions/user.tsx

+ * @param {string} onFail - функция для обработки ошибок
+ */
+
+// TODO что делать с options и getRequest???


Ну у options есть специальный тип RequestInit, но не вижу типизировать. getRequest - Тут только дженериками (с extends) решать вопрос))

ArslanMustafin · 2023-04-03T05:44:50Z

src/services/reducers/ws.tsx

+  connectionError: ''
+}
+
+export const wsReducer = createReducer(initialWSState as TWSState, (builder: any) => {


Надо исправить:

здесь и далее.

Правильно, что ts ругается, не нужно указывать везде тип (у state, builder и т.п.), тут всё само подхватывается.
А ругается ts потому что у вас расходится тип TWSState, он не должен состоять из нескольких.

ArslanMustafin · 2023-04-03T06:07:13Z

src/components/AppHeader/AppHeader.tsx

+  const active: string = AppHeaderStyles.link + ' ' + AppHeaderStyles.active + ' pl-5 pt-4 pr-5 pb-4 text text_type_main-default';
+  const link: string = AppHeaderStyles.link + ' pl-5 pt-4 pr-5 pb-4 text text_type_main-default text_color_inactive';
+  const activeAccountLink: string = AppHeaderStyles.link + ' ' + AppHeaderStyles.active + ' ' + AppHeaderStyles.account + ' pl-5 pt-4 pr-5 pb-4 text text_type_main-default';
+  const accountLink: string = AppHeaderStyles.link + ' ' + AppHeaderStyles.account + ' pl-5 pt-4 pr-5 pb-4 text text_type_main-default text_color_inactive';


Можно лучше:

Тут не обязательно указывать тип, т.к. ts сам подхватывает литеральные (простые типы) типы;

ArslanMustafin · 2023-04-03T06:07:43Z

src/components/BurgerConstructor/BurgerConstructor.tsx



-function BurgerConstructor({ openOrderDetailsModal }) {
+const BurgerConstructor: FC<{openOrderDetailsModal: () => void}> = ({ openOrderDetailsModal }) => {


Можно лучше:

лучше вынести описание пропсов, в отдельный тип.

ArslanMustafin · 2023-04-03T06:10:15Z

src/components/Ingredient/Ingredient.tsx

-    const targetElement = e.target.parentNode.parentNode;
+  // TODO
+  const handleDeleteIngredient = (e: MouseEvent<HTMLLIElement>) => {
+    console.log('e', e.target);


Надо исправить:

В приложении не должно быть консоль логов или дебаггеров. Вместо console.log используйте console.error() для ошибок.

ArslanMustafin · 2023-04-03T06:14:08Z

src/components/Orders/Orders.tsx

+  const { total, totalToday, orders } = useSelector((state) => (
+    state.ws.status === WS_STATUS_ONLINE && state.ws.feed?.success ? state.ws.feed as TFeedData : {orders: [], total: 0, totalToday: 0}
+  ));


Надо исправить:

вы не должны тут сами задавать default значения полей, это всё должно быть в редьюсере.

ArslanMustafin · 2023-04-03T06:15:38Z

src/components/TotalPrice/TotalPrice.tsx

+
+const TotalPrice: FC = () => {
+
+  const { addedIngredients: { bun, others } }: TMenuState = useSelector((state: TRootState) => state.menu);


Надо исправить:

вы типизировали useSelector, поэтому не стоит дополнительно указывать тип TMenuState.

ArslanMustafin · 2023-04-03T06:18:09Z

src/services/actions/ingredients.tsx

+      } else {
+        // Пробрасывать ошибку и обрабатывать все только в catch
+        dispatch(getIngredientsFailedAction());
+      }


Тут можно просто вызывать ошибку - throw new Error("Message")

ArslanMustafin · 2023-04-03T06:19:44Z

src/utils/constants.tsx

+export const WS_STATUS_OFFLINE: 'OFFLINE' = 'OFFLINE';
+export const WS_STATUS_ONLINE: 'ONLINE' = 'ONLINE';
+export const WS_STATUS_CONNECTING: 'CONNECTING...' = 'CONNECTING...';
+
+export const WS_MESSAGE: 'WS_MESSAGE' = 'WS_MESSAGE';
+export const WS_CONNECT: 'WS_CONNECT' = 'WS_CONNECT';
+export const WS_DISCONNECT: 'WS_DISCONNECT' = 'WS_DISCONNECT';
+export const WS_CONNECTING: 'WS_CONNECTING' = 'WS_CONNECTING';
+export const WS_OPEN: 'WS_OPEN' = 'WS_OPEN';
+export const WS_CLOSE: 'WS_CLOSE' = 'WS_CLOSE';
+export const WS_ERROR: 'WS_ERROR' = 'WS_ERROR';
+
+// Константы для обработки запроса для получения всех ингридиентов
+export const GET_INGREDIENTS_REQUEST: 'GET_INGREDIENTS_REQUEST' = 'GET_INGREDIENTS_REQUEST';
+export const GET_INGREDIENTS_SUCCESS: 'GET_INGREDIENTS_SUCCESS' = 'GET_INGREDIENTS_SUCCESS';
+export const GET_INGREDIENTS_FAILED: 'GET_INGREDIENTS_FAILED' = 'GET_INGREDIENTS_FAILED';
+
+// Константы для обработки запроса оформления заказа
+export const SEND_ORDER_REQUEST: 'SEND_ORDER_REQUEST'= 'SEND_ORDER_REQUEST';
+export const SEND_ORDER_SUCCESS: 'SEND_ORDER_SUCCESS' = 'SEND_ORDER_SUCCESS';
+export const SEND_ORDER_FAILED: 'SEND_ORDER_FAILED' = 'SEND_ORDER_FAILED';
+
+// Константы для получения/удаления данных об отдельном ингридиенте
+export const SHOW_INGREDIENT: 'SHOW_INGREDIENT' = 'SHOW_INGREDIENT';
+export const HIDE_INGREDIENT: 'HIDE_INGREDIENT' = 'HIDE_INGREDIENT';
+
+// Константы для добавления ингридиента в конструктор бургера
+export const ADD_INGREDIENT: 'ADD_INGREDIENT' = 'ADD_INGREDIENT';
+export const DELETE_INGREDIENT: 'DELETE_INGREDIENT' = 'DELETE_INGREDIENT';
+export const ADD_BUN: 'ADD_BUN' = 'ADD_BUN';
+
+// Константы для сортировки ингридиентов бургера
+export const CHANGE_INGREDIENT_ORDER: 'CHANGE_INGREDIENT_ORDER' = 'CHANGE_INGREDIENT_ORDER';
+
+// Константа для изменения активного таба
+export const SET_ACTIVE_TAB: 'SET_ACTIVE_TAB' = 'SET_ACTIVE_TAB';
+
+// Константы для обработки запроса получения данных о пользователе
+export const SET_REGISTER_FORM_VALUE: 'SET_REGISTER_FORM_VALUE' = 'SET_REGISTER_FORM_VALUE';
+export const SET_EDIT_USER_FORM: 'SET_EDIT_USER_FORM' = 'SET_EDIT_USER_FORM';
+
+export const ADD_USER_REQUEST: 'ADD_USER_REQUEST' = 'ADD_USER_REQUEST';
+export const ADD_USER_SUCCESS: 'ADD_USER_SUCCESS' = 'ADD_USER_SUCCESS';
+export const ADD_USER_FAILED: 'ADD_USER_FAILED' = 'ADD_USER_FAILED';
+
+export const LOGIN_USER_REQUEST: 'LOGIN_USER_REQUEST' = 'LOGIN_USER_REQUEST';
+export const LOGIN_USER_SUCCESS: 'LOGIN_USER_SUCCESS' = 'LOGIN_USER_SUCCESS';
+export const LOGIN_USER_FAILED: 'LOGIN_USER_FAILED' = 'LOGIN_USER_FAILED';
+
+export const LOGOUT_USER_SUCCESS: 'LOGOUT_USER_SUCCESS' = 'LOGOUT_USER_SUCCESS';
+export const LOGOUT_USER_FAILED: 'LOGOUT_USER_FAILED' = 'LOGOUT_USER_FAILED';
+
+export const GET_USER_SUCCESS: 'GET_USER_SUCCESS' = 'GET_USER_SUCCESS';
+export const GET_USER_FAILED: 'GET_USER_FAILED' = 'GET_USER_FAILED';
+
+export const EDIT_USER_SUCCESS: 'EDIT_USER_SUCCESS' = 'EDIT_USER_SUCCESS';
+export const EDIT_USER_FAILED: 'EDIT_USER_FAILED' = 'EDIT_USER_FAILED';
+export const RESET_EDIT_USER_FORM: 'RESET_EDIT_USER_FORM' = 'RESET_EDIT_USER_FORM';
+
+export const FORGOT_PASSWORD_SUCCESS: 'FORGOT_PASSWORD_SUCCESS' = 'FORGOT_PASSWORD_SUCCESS';
+export const FORGOT_PASSWORD_FAILED: 'FORGOT_PASSWORD_FAILED' = 'FORGOT_PASSWORD_FAILED';
+
+export const RESET_PASSWORD_SUCCESS: 'RESET_PASSWORD_SUCCESS' = 'RESET_PASSWORD_SUCCESS';
+export const RESET_PASSWORD_FAILED: 'RESET_PASSWORD_FAILED' = 'RESET_PASSWORD_FAILED';
+
+export const GET_USER_ORDERS: 'GET_USER_ORDERS' = 'GET_USER_ORDERS';


Можно лучше:

Не стоит явно указывать тип у литеральных констант, ведь такие константы, по умолчанию имеют тип равный её значению.

ArslanMustafin

Code Review

Здравствуйте!

Отличная работа.

Вы молодец! Работа принята, удачи в дальнейшем обучении!

Все замечания были исправлены;

Можно лучше:

По идее https://disk.yandex.com/i/hlr47aYC3cOuhQ - этот файл тоже нужно типизировать, я что-то пропустил его в первое ревью, но думаю вы с ним справитесь.

ulbee added 12 commits March 29, 2023 03:00

Типизация части компонентов и констант, не требующих redux

7fe35f7

Общие модули работы с куками и датами переведены на typescript

4282658

Первая версия типизации экшнов и апи

0ae4e81

Типизация экшнов и редьюсеров для вебсокет-соединения, доработки для …

016aec3

…хранилища даннях пользователя и ингридиентов

В services вынесены store и типы user, ingredients и ws

7cc598d

Типизация страниц

0c6de22

Фикс ошибок типизации вебсокет-соединения

63980d4

Фикс ошибок типизации для восстановления пароля; типизация АПИ

4b044ec

Типизация части компонентов

a073513

Типизация компонентов для отображения заказов

6526381

Доработки типизации компонентов для отображения заказа

d36e8dd

Фикс ошибок типизации компонентов и хранилища

7db32f6

ArslanMustafin suggested changes Apr 3, 2023

View reviewed changes

Правки по ревью

0e6d389

ArslanMustafin approved these changes Apr 3, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Month10/step1#8

Month10/step1#8
ulbee wants to merge 13 commits intomainfrom
month10/step1

ulbee commented Apr 2, 2023

Uh oh!

ArslanMustafin left a comment

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin Apr 3, 2023

Uh oh!

ArslanMustafin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		function BurgerConstructor({ openOrderDetailsModal }) {
		const BurgerConstructor: FC<{openOrderDetailsModal: () => void}> = ({ openOrderDetailsModal }) => {


		const TotalPrice: FC = () => {

		const { addedIngredients: { bun, others } }: TMenuState = useSelector((state: TRootState) => state.menu);

Conversation

ulbee commented Apr 2, 2023

Uh oh!

ArslanMustafin left a comment

Choose a reason for hiding this comment

Code Review

Вы молодец!

Но есть несколько замечаний, которые нужно исправить:

Сделает код лучше:

Подробно эти и более мелкие замечания, отмечены в коде.

После исправления всех замечаний, работа будет принята)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArslanMustafin left a comment

Choose a reason for hiding this comment

Code Review

Вы молодец! Работа принята, удачи в дальнейшем обучении!

Можно лучше:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants