search

feature-sliced / documentation

🍰 Architectural design methodology for Frontend projects
https://feature-sliced.design
MIT License
1.4k stars 146 forks source link

Add new example - Falkchat #648

Closed falkomerr closed 5 months ago

falkomerr commented 6 months ago

Пример с использованием shadcn/ui и Next

https://github.com/falkomerr/falkchat

netlify[bot] commented 6 months ago

Deploy Preview for pr-fsd ready!

Name Link
Latest commit 1eebceb01e54348908af9cbde0be66a032dac5de
Latest deploy log https://app.netlify.com/sites/pr-fsd/deploys/65856d5ad7cf6f0008a5ea19
Deploy Preview https://deploy-preview-648--pr-fsd.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

falkomerr commented 6 months ago

Привет, принял правки.

Что касается pages, то это скорее не про FSD а про сокеты, они не работают с новым App Router из-за чего их нужно помещать в папку pages.

illright commented 5 months ago

Привет еще раз, спасибо, по тем правкам все хорошо. Посмотрел проект прямо подробно, выписал следующие пункты:

  1. hooks — не очень хорошее название для сегмента (речь про shared/hooks), потому что описывает не техническое назначение (зачем это), а суть (что это). Я вижу там хуки, влияющие на интерфейс (use-chat-scroll.ts), взаимодействующие с сервером (use-chat-socket.ts) и с хранилищем (use-modal-store.ts). Им бы быть в соответствующих сегментах (ui, api, store соответственно), а не вместе, потому что их мало что связывает. Тот же комментарий про shared/utils. utils — очень привлекательное название, но не очень полезное, потому что никак не помогает понять, что там можно найти, и часто превращается в свалку кода. Такое лучше унести к месту использования, либо разбить на более описательные сегменты.

  2. Заметил, что в shared/hooks у тебя есть папка lib, которая как бы намекает, что hooks — это слайс, а не сегмент. В shared не должно быть слайсов, потому что слайсы делят по предметной области, а в shared лежит общий код для всех областей. Аналогично с shared/providers/ui (не говоря о том, что провайдеры — не особо UI-штука, а скорее, что-то для API или хранилища).

  3. Есть вопросы к некоторым сущностям. В целом, сущности стоит заводить только тогда, когда есть несколько слайсов на слоях выше, которые работают с ними. Таким образом, сущности emoji, message, navigation и socket сложно оправдать. Проблема с тем, чтоб заводить лишние сущности в том, что люди в команде могут не знать о их существовании и писать код, который стоило бы поместить в сущность, где-то ещё. В целом, по опыту могу сказать, что выделение сущностей — довольно сложно, поэтому ожидать от всех людей умение это делать не приходится. Рекомендую перенести эти сущности в место использования.

  4. Аналогичный поинт про фичи, но в случае с фичами все ещё сложнее, и поэтому рекомендации настоятельнее. Каждая из фич используется только в одном слайсе выше, поэтому нет преимущества реюза кода, но при этом архитектура сложнее для понимания. Если фич и сущностей в проекте нет (абсолютно нормальная ситуация), то FSD, по сути, не требует дополнительного понимания, делить на страницы, виджеты, и переиспользуемый код умеют все. Если в проект легко вкатиться и начать приносить пользу, то архитектура сделала свое дело. Рекомендую убрать фичи и перенести код выше.

  5. Слайс widgets/modals кажется мне очень перегруженным. Опять же, группировка там по сути (что это — модалка), а не по назначению (зачем это — чтоб создать сервер). Рекомендую какие-то модалки унести поближе к страницам, а каким-то выделить отдельный виджет.

  6. Слой pages очень хороший, думаю, грех терять его из-за Некста. Там в доке есть предложение добавить папку pages в корне, чтоб src/pages осталась за FSD. Когда фреймворк использует файловый роутинг, обычно бывает трудно понять, какие страницы вообще есть в проекте (мне, например, тут трудно), плоская структура страниц в FSD с этим очень помогает.

Еще местами заметил, что у тебя импорты залезают внутрь слайса, например, в src/app/(main)/(routes)/servers/[serverId]/conversations/[memberId]/page.tsx (ну и имечко у файла блин).

falkomerr commented 5 months ago

Привет, поправил.

  1. Разгрузил слайс widgets/modals
  2. entities и features, которые использовались в одном месте, перенёс туда без декомпозиции.
  3. Разбил hooks и utils на несколько сегментов и те функции, которые использовались единожды, перенёс в те места где они используются.
  4. Вынес страницы в pages(пришлось переименовать в client-pages, чтобы не конфликтовать с некстом)
illright commented 5 months ago

suggestion (non-blocking): Ты назвал сегмент в Shared api-functions. Это нормальное и понятное название, но в тех случаях, когда у FSD есть общепринятые названия (ui для UI-кита, api для запросов, config для енв-переменных), рекомендуется использовать их, чтоб снизить фрикцию вкатывания в проект.

issue: В shared/providers/modal-provider ты импортируешь из виджетов, что запрещено методологией. suggestion: В целом, FSD рекомендует хранить провайдеры на слое App. У некста есть с этим проблемы определенные, да, но если назвать сегмент _providers, то Некст не обратит на него внимание. Если перенесешь, возникнут проблемы с socket-provider, их можно пофиксить, оставив объект контекста в shared/socket, а провайдер перенести вверх.

issue: сегмент shared/use-chat немного "нематериальный" — мне не очень понятно, для чего этот сегмент нужен, какой код в дальнейшем туда можно класть, а какой нельзя. Более того, он фактически знает об устройстве виджета чата, например, хук useChatScroll принимает chatRef и bottomRef, то есть он заточен под определенную структуру. suggestion: Заметил, что он используется только в виджете чата, мб его туда и отправить? Не думаю, что есть смысл ему оставаться в Shared.

issue: с сущностью message тоже не все гладко, потому что некоторые компоненты в ней — это прямо интерактив, и более того, код в этой сущности, опять же, знает о том, что на слоях сверху будет создана модалка deleteMessage, в которую будет зашита бизнес-логика. В идеале, на слое сущностей особо бизнес-логики быть не должно, и особенно трудно такое отловить, когда зависимости неявные, например, как в случае с модалками — импорта нет, а зависимость есть. suggestion: советую выпилить нахрен сущность message, потому что она используется только в одном слайсе, то есть, она не служит цели реюза кода. Если это не вариант (и в целом, когда такие ситуации на слое сущностей возникают), имеет смысл сделать слоты для интерактивных элементов (например, добавить проп, в который будет прокинут JSX).

suggestion (non-blocking): у тебя есть фича file, рекомендую переименовать ее в upload-file. Фичи — это обычно взаимодействия, поэтому имеет смысл называть их через глаголы.

issue: немного путаюсь в features/chat, widgets/chat, shared/use-chat, client-pages/chat-id. Если бы мне нужно было изменить что-то в функциональности чата, я бы прошерстил это все, прежде чем начать кодить, а это не очень эффективно suggestion: думаю, фичу chat нужно переименовать поподробнее, потому что даже chat-input мне особо не говорит, что это за фича, где ее зона ответственности, что туда можно добавлять, а что нельзя. Про shared уже говорил, остальное, в принципе, можно оставить.

question: разве widgets/modals/ui/message-attachment-modal не относится к виджету чата?

nitpick (non-blocking): в src/client-pages/invite-page/ui/invite-page.tsx:57 есть return <div>wkdwdwk</div>;, лол, не знаю, это намеренно или нет

question: а почему ты решил назвать слой pages client-pages? Вроде, если создать /pages, то /src/pages не будет учтен Некстом (могу ошибаться, сам не пробовал). В любом случае, если слои называются нестандартно, то это стоит задокументировать в ридми проекта, чтоб люди, приходящие в проект (и знающие FSD) могли сразу сориентироваться

falkomerr commented 5 months ago

  1. переименовал в api-helpers, так как api как я понимаю подходит для конкретных запросов к бекенду, а тут просто вспомогательные функции(которые к тому же зачастую выполняются на сервере, а не на клиенте).

  2. Перенес провайдеры в app/_providers, socketContext и useSocket оставил в shared/socket.

  3. Перенес useChatScroll в виджет чата, а useChatQuery и useChatSocket оставил в shared, так как они работают с сокетами и react-query.

  4. Удалил слайс messages и перенес сегменты в нужные места.

  5. переименовал features/file -> features/upload-file

  6. переименовал chat в message а chat-input в message-input, а остальные компоненты внутри сегмента ui перенес в места использования

  7. перенёс

  8. поставил затычку когда страницу писал и забыл, сейчас убрал

  9. да, действительно, если создать pages в корне то некст не будет обращать внимание на src/pages