React.js оцените код/приложуху

В основном прогал всегда бэк (в том числе и на node.js) и фронт пытался избегать стороной, где-то 4 дня назад решил изучить react

вот вчера - сегодня за ~8часов набросал это с ts mui react-router-dom (redux тож немного копнул, но для этого прожекта это юзлесс) https://r1tsuu.github.io/github-search/ https://github.com/r1tsuu/github-search/tree/master/src (правда поиск по репам пока что не запилил и поиск по страницам)

 

 

GitHub pages не работает. 

А по коду: гуд, чистенько, правда смотреть не на что совсем

Buhalo сказал(а):↑

GitHub pages не работает. 

А по коду: гуд, чистенько, правда смотреть не на что совсем

Нажмите, чтобы раскрыть...

работает я обновил ссылку

когда обновляешь страницу редиректит с .io/github-search/ на .io/ не особо понимаю поч ))

Ritsuko сказал(а):↑

работает я обновил ссылку

когда обновляешь страницу редиректит с .io/github-search/ на .io/ не особо понимаю поч ))

Нажмите, чтобы раскрыть...

Особенности реакта на gh-pages, забей 

Ну работает, конечно мое фронтендерское сердце разбивается об косяки верстки (смотрел на мобиле)

Ritsuko сказал(а):↑

В основном прогал всегда бэк (в том числе и на node.js) и фронт пытался избегать стороной, где-то 4 дня назад решил изучить react

вот вчера - сегодня за ~8часов набросал это с ts mui react-router-dom (redux тож немного копнул, но для этого прожекта это юзлесс) https://r1tsuu.github.io/github-search/ https://github.com/r1tsuu/github-search/tree/master/src (правда поиск по репам пока что не запилил и поиск по страницам)

 

 

Нажмите, чтобы раскрыть...

 

По js не буду придираться, вряд ли это твой мейн язык, но по самому реакту немного предложений:

 

- Запросы на бэк из компонентов лучше не делать, если подключил редакс то запросы делай оттуда, сами запросы лучше вынести в отдельный класс (папка services) и т.к сервис у тебя один, можешь просто создать класс Api со статическими полями, который на каждый метод будет посылать запросы, а из редакса просто юзай например Api.fetchUsers().

- Страницы и компоненты лучше выноси в отдельные папки, потому что они имеют свои стили, интерфейсы и index.tsx файл

- От инлайновых сталей лучше откажись, подключи styled-components

- Для импортов можешь алиасы настроить, и импортировать компоненты так - from '@components/example'

- React.Fragment'ы замени на <>>

- С именованиями не совсем понятен префикс "My" у некоторых компонентов

- Ещё не совсем понятно почему общий файл с типами называется types.d.ts, если 'd.ts' используется для деклараций, переименуй в types.ts

Когда несколько найдено

 

Когда найден один

 

 

 

 

Aragorn_ сказал(а):↑

Когда несколько найдено

 

Когда найден один

 

 

 

Нажмите, чтобы раскрыть...

я когда с 1 файла все на разные расставлял походу забыл для айтема юзеров поставить на всю ширину

ща норм

Спойлер

 

Ritsuko сказал(а):↑

я когда с 1 файла все на разные расставлял походу забыл для айтема юзеров поставить на всю ширину

ща норм

Спойлер

 

Нажмите, чтобы раскрыть...

Теперь добавь бесконечную пагинацию)

сомневаюсь что на популярные ключевые слова находит 30-40 аккаунтов)

Aragorn_ сказал(а):↑

Теперь добавь бесконечную пагинацию)

сомневаюсь что на популярные ключевые слова находит 30-40 аккаунтов)

Нажмите, чтобы раскрыть...

да гитхаб апи по 30 акков отправляет на каждую страницу, если не указывать страницу получается что толшько 1

Ritsuko сказал(а):↑

да гитхаб апи по 30 акков отправляет на каждую страницу, если не указывать страницу получается что толшько 1

Нажмите, чтобы раскрыть...

ну вот, теперь прихольная задача добавить страницы)

Justp1ayer сказал(а):↑

 

По js не буду придираться, вряд ли это твой мейн язык, но по самому реакту немного предложений:

 

- Запросы на бэк из компонентов лучше не делать, если подключил редакс то запросы делай оттуда, сами запросы лучше вынести в отдельный класс (папка services) и т.к сервис у тебя один, можешь просто создать класс Api со статическими полями, который на каждый метод будет посылать запросы, а из редакса просто юзай например Api.fetchUsers().

Нажмите, чтобы раскрыть...

Не надо подключать редакс никогда только для того, чтобы делать оттуда запросы, ради бога.

 

Justp1ayer сказал(а):↑

- От инлайновых сталей лучше откажись, подключи styled-components

Нажмите, чтобы раскрыть...

css-in-js - штука довольно спорная, особенно для SPA без роутинга

Ixfus сказал(а):↑

css-in-js - штука довольно спорная, особенно для SPA без роутинга

Нажмите, чтобы раскрыть...

Почему? 

Ritsuko сказал(а):↑

В основном прогал всегда бэк (в том числе и на node.js) и фронт пытался избегать стороной, где-то 4 дня назад решил изучить react

вот вчера - сегодня за ~8часов набросал это с ts mui react-router-dom (redux тож немного копнул, но для этого прожекта это юзлесс) https://r1tsuu.github.io/github-search/ https://github.com/r1tsuu/github-search/tree/master/src (правда поиск по репам пока что не запилил и поиск по страницам)

 

 

Нажмите, чтобы раскрыть...

Импорты сделай абсолютным с корневой попкой src

В индекс тс файле обычно ничего нет, обёртка браузердома должны быть в Апп (вкусовщина)

Ну и по юзер экспириенсу если расписывать все минусы приложения то их будет довольно много, но я думаю ты и сам видишь

 

InversionSpells сказал(а):↑

Почему? 

Нажмите, чтобы раскрыть...

Потому что увеличивает и без того пухлый js-бандл, увеличивая время первой отрисовки и Time to Action.

Хотя со стайлдами JSX выглядит, конечно, почище.

Ritsuko сказал(а):↑

В основном прогал всегда бэк (в том числе и на node.js) и фронт пытался избегать стороной, где-то 4 дня назад решил изучить react

вот вчера - сегодня за ~8часов набросал это с ts mui react-router-dom (redux тож немного копнул, но для этого прожекта это юзлесс) https://r1tsuu.github.io/github-search/ https://github.com/r1tsuu/github-search/tree/master/src (правда поиск по репам пока что не запилил и поиск по страницам)

 

Нажмите, чтобы раскрыть...

Смотрел только код, из того, что не назвали.

1. Компоненты не типизированы.

2. Вызов аксиоса происходит не в useEffect

3. Обычно для компонентов создаются папочки, где лежат 1) сам компонент 2) стили 3) интерфейсы пропсов 4) опционально index для коротких импортов.

aQuere сказал(а):↑

Импорты сделай абсолютным с корневой попкой src

Нажмите, чтобы раскрыть...

Это зачем?

Ixfus сказал(а):↑

Потому что увеличивает и без того пухлый js-бандл, увеличивая время первой отрисовки и Time to Action.

Хотя со стайлдами JSX выглядит, конечно, почище.

 

Смотрел только код, из того, что не назвали.

1. Компоненты не типизированы.

2. Вызов аксиоса происходит не в useEffect

3. Обычно для компонентов создаются папочки, где лежат 1) сам компонент 2) стили 3) интерфейсы пропсов 4) опционально index для коротких импортов.

 

Это зачем?

Нажмите, чтобы раскрыть...

Смотрится лучше при большой вложенности файла в который импортируешь

aQuere сказал(а):↑

Смотрится лучше при большой вложенности файла в который импортируешь

Нажмите, чтобы раскрыть...

там же будет огромное полотно, вместо ../../../

Ixfus сказал(а):↑

там же будет огромное полотно, вместо ../../../

Нажмите, чтобы раскрыть...

Или же наоборот components/Header вместо

../../../../../Header

Абсолютные импорты смотрятся лучше

aQuere сказал(а):↑

Или же наоборот components/Header вместо

../../../../../Header

Абсолютные импорты смотрятся лучше

Нажмите, чтобы раскрыть...

Все равно в ide они скрываются, так шо

Ixfus сказал(а):↑

Все равно в ide они скрываются, так шо

Нажмите, чтобы раскрыть...

Не во всех, смотря как настроишь

Плюс если говорить про командную разработку - это стандарт (абс имп)

Justp1ayer сказал(а):↑

 

По js не буду придираться, вряд ли это твой мейн язык, но по самому реакту немного предложений:

 

- Запросы на бэк из компонентов лучше не делать, если подключил редакс то запросы делай оттуда, сами запросы лучше вынести в отдельный класс (папка services) и т.к сервис у тебя один, можешь просто создать класс Api со статическими полями, который на каждый метод будет посылать запросы, а из редакса просто юзай например Api.fetchUsers().

- Страницы и компоненты лучше выноси в отдельные папки, потому что они имеют свои стили, интерфейсы и index.tsx файл

- От инлайновых сталей лучше откажись, подключи styled-components

- Для импортов можешь алиасы настроить, и импортировать компоненты так - from '@components/example'

- React.Fragment'ы замени на <>>

- С именованиями не совсем понятен префикс "My" у некоторых компонентов

 

- Ещё не совсем понятно почему общий файл с типами называется types.d.ts, если 'd.ts' используется для деклараций, переименуй в types.ts

Нажмите, чтобы раскрыть...

Спасибо. Сейчас пишу с редаксом уже магаз, вот интересно, писать так компоненты - норм (через content = useState())? увидел подобное в редакс доках просто

единственное что хз как правильней, useEffect один и проверять там все статусы, или так по 1. (на картинке для failed и succeeded забыл 2ой арг useEffect)

А зачем ты каждый компонент страницы оборачиваешь в React.Fragment? В этом нет никакого смысла, учитывая, что у тебя только 1 children свегда. В целом - всё ок, единственное, конечно, не так уж и много кода у тебя написано, чтоб делать какие-то выводы.

 

Ещё хочу добавить, что если уж пишешь компоненты на TypeScript, явно объявляй, что твоя функция - это компонент:

 

const MyComponent: React.FC = () => { ... }

или

interface MyComponentProps {

  someProp: boolean;

}

const MyComponent: React.FC = () => { ... }

 

Yuzi сказал(а):↑

или

interface MyComponentProps {

  someProp: boolean;

}

const MyComponent: React.FC = () => { ... }

 

Нажмите, чтобы раскрыть...

тогда уж не const MyComponent: React.FC = (props: MyComponentProps)=> { ... }? 

 

Ritsuko сказал(а):↑

тогда уж не const MyComponent: React.FC = (props: MyComponentProps)=> { ... }? 

 

Нажмите, чтобы раскрыть...

Нет, потому что тип React.FC содержит ещё в себе информацию о children:

// Здесь TypeScript знает о том, что такое children

const MyComponent: React.FC = ({ children }) => { ... }

А твой вариант об этом не знает :)

П. С. Блин, движок форума обрубает HTML теги

 

И ещё, кстати, зачем эти махинации с setContent?

Ты ж спокойно можешь написать:

 

Т. к. в useEffect мы не передали никаких зависимостей, то хук выполнит коллбек только 1 раз (т. е. при маунте страницы), ну а по рендеру там понятно дальше, декларативно и меньше кода :)

Yuzi сказал(а):↑

Нет, потому что тип React.FC содержит ещё в себе информацию о children:

// Здесь TypeScript знает о том, что такое children

const MyComponent: React.FC = ({ children }) => { ... }

А твой вариант об этом не знает :)

П. С. Блин, движок форума обрубает HTML теги

 

И ещё, кстати, зачем эти махинации с setContent?

Ты ж спокойно можешь написать:

 

Т. к. в useEffect мы не передали никаких зависимостей, то хук выполнит коллбек только 1 раз (т. е. при маунте страницы), ну а по рендеру там понятно дальше, декларативно и меньше кода :)

Нажмите, чтобы раскрыть...

Я вкурсе про это. Просто я смотрю разные видосы, смотрю например документации (в редакс доке такой пример) и везде все по разному как-то относятся к тому, как использовать хуки и т.д. Наверное единого варианта написания кода найти

невозможно

 

Вот в чем наверное главный плюс бэка перед фронтом - заучил паттерны, пилишь себе на каждый чих двадцать геттеров и сеттеров и все так делают.

 

Ritsuko сказал(а):↑

Спасибо. Сейчас пишу с редаксом уже магаз, вот интересно, писать так компоненты - норм (через content = useState())? увидел подобное в редакс доках просто

единственное что хз как правильней, useEffect один и проверять там все статусы, или так по 1. (на картинке для failed и succeeded забыл 2ой арг useEffect)

Нажмите, чтобы раскрыть...

 

Во первых, по поводу верхнего комментария от чувака, не надо пожалуйста строго указывать React.FC, потому что TS достаточно умён, и уже всё знает (можешь мышкой навести на переменную компонента и увидишь что там соответствующий Return type);

 

По поводу useEffect'ов, сделай отдельную функцию, например setContentByType, где через switch case исходя из статуса будешь производить определённые манипуляции, а юзэффект получается оставь один, и получится:

useEffect(setContentByType, [status]);

 

 

По поводу хранения контента в useState ничего плохого не вижу, только конечный setContent на скрине вынеси в отдельный компонент, как это сделано со Spinner и Loading, а ещё типизируй, а в качестве initialState не используй строковые значения, а если нужна пустота используй null.

Итого получится так:

 

const [content, setContent] = useState < (форум почему-то не парсит стрелочки) ReactElement | null > (null)

 

 

 

 

А вообще, по хорошему, всё что я написал выше это бред, и переменная content здесь лишняя, для лучшей архитектуры в корне компонента всё что тебе нужно - один switch case который вернёт JSX element исходя из status'a либо null, и всё, код сократится в 2 раза)