31Vector31 / react-training

Hooks, Axios, HOC, Redux, Context, Reselect, Router.
0 stars 0 forks source link

review AdminPanelHook #6

Closed shevchuknine closed 2 years ago

shevchuknine commented 2 years ago

https://github.com/31Vector31/react-training/blob/7ab7bc4de5cdd3ed371b9eac5178bfab4242edfb/src/AdminPanelHooks/AdminPanel.js#L8 key ведь и так уже в глобальной области и ты можешь досутчаться до него из любой функции, так что не имеет смысла передавать его в качестве аргумента колбеки, которые передаются в другие компоненты, не обернуты в useCallback https://github.com/31Vector31/react-training/blob/7ab7bc4de5cdd3ed371b9eac5178bfab4242edfb/src/AdminPanelHooks/AdminPanel.js#L81 внутри формы отличить режим редактирования от создания можно по пропсам username и department, делать им проверку на undefined. замечания про распыление, деструктуризацию и колбеки актуально и здесь тоже https://github.com/31Vector31/react-training/blob/7ab7bc4de5cdd3ed371b9eac5178bfab4242edfb/src/AdminPanelHooks/Form.js#L16 переменная незменна далее, можно использовать const _ https://github.com/31Vector31/react-training/blob/7ab7bc4de5cdd3ed371b9eac5178bfab4242edfb/src/AdminPanelHooks/Table.js#L11 имеет смысл использовать useMemo, т.к. пересчет необходим только в случае, если serach изменился

shevchuknine commented 2 years ago

https://github.com/31Vector31/react-training/blob/dd60f1a49d08cf8ade69c260cf9a19fdbfdfefa0/src/AdminPanelHooks/AdminPanel.js#L26 лишняя обертка над функцией https://github.com/31Vector31/react-training/blob/dd60f1a49d08cf8ade69c260cf9a19fdbfdfefa0/src/AdminPanelHooks/AdminPanel.js#L35 использование useCallback без зависимостей не приводит к улучшению производительности, т.к. функция все еще пересоздается каждый раз https://github.com/31Vector31/react-training/blob/dd60f1a49d08cf8ade69c260cf9a19fdbfdfefa0/src/AdminPanelHooks/AdminPanel.js#L72 эта обертка не нужна, т.к. не добавляет никакого функционала, ты можешь просто передавать setIdEditElement вместо editUser _ https://github.com/31Vector31/react-training/blob/dd60f1a49d08cf8ade69c260cf9a19fdbfdfefa0/src/AdminPanelHooks/Form.js#L12 если у пользователей будут идентичные имена, но разные отделы, это не сработает

shevchuknine commented 2 years ago

https://github.com/31Vector31/react-training/blob/c77bca1a8e045bd7d14d70200d590b02e904a72f/src/AdminPanelHooks/AdminPanel.js#L26 изначально идея правильная, передать фунцию в useState чтобы не вызывать обращение в LS каждый раз. обертка не нужна была потому, что не добавляла никакого функционала. ты можешь просто передавать localStorageGetItem и получишь тот же результат, поскольку это функция: useState(localStorageGetItem) _ https://github.com/31Vector31/react-training/blob/c77bca1a8e045bd7d14d70200d590b02e904a72f/src/AdminPanelHooks/AdminPanel.js#L75-L77 можно сократить до одной строчки при сохранении читабельного вида