-
Notifications
You must be signed in to change notification settings - Fork 174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new example to v2 list #721
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for pr-fsd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the example! There are no issues found with Steiger, great job! Looking at the project code, I have a few remarks:
- I often see
export * from
, for example, infront/src/shared/api/index.ts
. It's usually bad practice for defining public APIs, because the whole point of the public API is to restrict what objects are accessible outside of a slice. Wildcard exports make the public API less dependable and more difficult to read - The login modal is located in
entities/user
, which I found quite unexpected. I would expect to find it in a page or maybe a widget.
In general, I think that this application is small enough that having both Entities and Features is probably more distracting than beneficial to the architecture. I would suggest just having two widgets for the cat list and the auth modal, two pages, and then just the Shared layer.
Привет, @illright! По поводу замечаний:
|
Не думаю, что это будет правильно с точки зрения FSD. Слой Shared должен быть просто связующим звеном твоего приложения и внешнего мира, а такую логику, как открытие модалки по неудавшемуся запросу, я бы туда класть не стал. Более того, такая привязка может создать больше проблем в будущем, когда логика приложения будет расти. Я бы предложил решить эту проблему на стороне дизайна интерфейса — сделать четко зафиксированные триггеры для показывания этой модалки — например, нажатие на кнопку "логин" или нажатие на сердечко, когда пользователь не залогинен |
Привет!
|
Привет, спасибо!
Что с этим можно сделать: вижу, что cats-list использует из этой фичи только |
Changelog
New example to v2 list.
Cat Pinterest where you can look at kittens and like them (=^・ω・^=)