feature-sliced / documentation

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

Vue3 pinia example #617

Closed hoachnt closed 8 months ago

hoachnt commented 10 months ago

Добавил пример с использованием pinia, так как возможно кому-то будет интересно composition api с pinia

netlify[bot] commented 10 months ago

Deploy Preview for pr-fsd failed.

Name Link
Latest commit d6e6a5149002a1f06c20c51e95263fd5e34cc862
Latest deploy log https://app.netlify.com/sites/pr-fsd/deploys/64f0d02cf54bfd0008771e2a
azinit commented 10 months ago

@feature-sliced/core

illright commented 10 months ago

Привет! Спасибо за пример на Vue, а то у нас все реакт да реакт.

Несколько заметок:

Скажи, что думаешь насчет этого фидбека

hoachnt commented 10 months ago

Привет! Спасибо за пример на Vue, а то у нас все реакт да реакт.

Несколько заметок:

* Заметил у тебя в Shared сегмент button. Довольно специфично, на мой взгляд, по канонам FSD в shared лежат уже сразу сегменты, поэтому на мой взгляд, `shared/ui/button.ts` было бы более знакомо для разработчиков, знающих FSD. Аналогично с `radio-group`.

* В entities/tasks/ui есть компонент, который заканчивается на -Page. Я не ожидал увидеть на слое Entities компонент страницы, да и судя по самому компоненту, это, скорее, карточка, а не целая страница. Кажется, этот компонент отвечает за режим детального просмотра задачи. Предлагаю переименовать его в `entities/task/ui/details.vue`

* В Widgets у тебя есть слайс `modal`. Тут наоборот очень общо, по названию не понятно, какой именно это модал. Если это общий модал без привязки к конкретному бизнес-сценарию, предлагаю переместить в Shared. В противном случае предлагаю добавить в имя детали этого сценария, например, `new-task-modal`.

* Вижу в Pages страницы, которые просто рендерят виджет в лейауте, например, `task-page`. Если страница достаточно простая, рекомендую не делать лишних виджетов, и скомпозировать сразу на слое Pages. Т.е. переместить код из слоя Widgets в слой Pages, а виджет удалить. Так проще будет, без абстракций ради абстракций. Важный принцип FSD -- pain-driven development, делаем что-то только тогда, когда не делать этого слишком больно.

* Заметил у тебя в проекте процессы. Процессы ныне считаются устаревшим слоем, и рекомендуется раскидать существующие процессы по слоям Features и App. В твоем случае, процессу swipe-gesture ничего кроме Shared не требуется, да и никакой бизнес-ценности для пользователя, имхо, этот процесс не представляет (просто улучшает пользовательский опыт). В этом случае предлагаю просто положить его в `shared/swipe-gesture`.

Скажи, что думаешь насчет этого фидбека

Думаю ваши замечания очень даже корректны. Ваше замечание насчёт "Не делать лишних виджетов", я не вижу проблем с этим, так как лучше сразу делать абстракцию, чтобы в дальнейшем его уже просто расширять. Хотя это противоречит принципу FSD. В целом буду потихоньку править код)

hoachnt commented 9 months ago

Привет! Спасибо за пример на Vue, а то у нас все реакт да реакт.

Несколько заметок:

* Заметил у тебя в Shared сегмент button. Довольно специфично, на мой взгляд, по канонам FSD в shared лежат уже сразу сегменты, поэтому на мой взгляд, `shared/ui/button.ts` было бы более знакомо для разработчиков, знающих FSD. Аналогично с `radio-group`.

* В entities/tasks/ui есть компонент, который заканчивается на -Page. Я не ожидал увидеть на слое Entities компонент страницы, да и судя по самому компоненту, это, скорее, карточка, а не целая страница. Кажется, этот компонент отвечает за режим детального просмотра задачи. Предлагаю переименовать его в `entities/task/ui/details.vue`

* В Widgets у тебя есть слайс `modal`. Тут наоборот очень общо, по названию не понятно, какой именно это модал. Если это общий модал без привязки к конкретному бизнес-сценарию, предлагаю переместить в Shared. В противном случае предлагаю добавить в имя детали этого сценария, например, `new-task-modal`.

* Вижу в Pages страницы, которые просто рендерят виджет в лейауте, например, `task-page`. Если страница достаточно простая, рекомендую не делать лишних виджетов, и скомпозировать сразу на слое Pages. Т.е. переместить код из слоя Widgets в слой Pages, а виджет удалить. Так проще будет, без абстракций ради абстракций. Важный принцип FSD -- pain-driven development, делаем что-то только тогда, когда не делать этого слишком больно.

* Заметил у тебя в проекте процессы. Процессы ныне считаются устаревшим слоем, и рекомендуется раскидать существующие процессы по слоям Features и App. В твоем случае, процессу swipe-gesture ничего кроме Shared не требуется, да и никакой бизнес-ценности для пользователя, имхо, этот процесс не представляет (просто улучшает пользовательский опыт). В этом случае предлагаю просто положить его в `shared/swipe-gesture`.

Скажи, что думаешь насчет этого фидбека

Проверь пожалуйста. Я исправил моменты, которые ты подметил

hoachnt commented 9 months ago

@feature-sliced/core

illright commented 8 months ago

Все гуд! Заметил у тебя еще наряду со слоями папку assets с иконкой vue, если не используется, удали, пожалуйста.

Насчет добавления примера в доку: там чето, кажется, не так с именем файла со скриншотом, можешь проверить еще раз? И заодно подлить изменения из master к себе в ветку, чтоб конфликт зарезолвить. И можем вливать

netlify[bot] commented 8 months ago

Deploy Preview for pr-fsd ready!

Name Link
Latest commit 854eddcd61493ff2293dad138a52e22954e29f4b
Latest deploy log https://app.netlify.com/sites/pr-fsd/deploys/65286341c23c1d0008f8b4ca
Deploy Preview https://deploy-preview-617--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.

hoachnt commented 8 months ago

Все гуд! Заметил у тебя еще наряду со слоями папку assets с иконкой vue, если не используется, удали, пожалуйста.

Насчет добавления примера в доку: там чето, кажется, не так с именем файла со скриншотом, можешь проверить еще раз? И заодно подлить изменения из master к себе в ветку, чтоб конфликт зарезолвить. И можем вливать

Можешь чекнуть. Вроде нет проблем)