feature-sliced / documentation

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

Add example of Money Flow mobile application #622

Closed astsu-dev closed 8 months ago

netlify[bot] commented 10 months ago

Deploy request for pr-fsd pending review.

Visit the deploys page to approve it

Name Link
Latest commit f952b30e56def2a7c08e5d3bfbbbd18d69387933
astsu-dev commented 10 months ago

@feature-sliced/core How long does it take to review an example?

illright commented 10 months ago

It's not so much about time as about willpower. When the core team has energy to review projects, they get reviewed quickly, but I can't give an estimate when that will happen

illright commented 8 months ago

Hey! Sorry it took so long to get to this project. I looked at it, and generally, I'm very impressed. Here are a few minor comments:

  1. I noticed you have separated API methods in shared/api by entity. With this division, I think it makes more sense if it's placed next to the entity itself, that way, everything about the entity is in one place
  2. [matter of preference] In your entities' UI you prefix component files with the name of the entity, this may be redundant, but also may be useful to navigate in the editor by tab names. Just something to think about, it's fine as it is

I'd particularly like to hear your thought on point 1. Other than that, the project is flawless, respect. I really like it and I think it makes for a great example of a larger project.

Btw I noticed the moneyflow.cash link just redirects to your GitHub, I expected to be able to try the app online. Is that working correctly?

astsu-dev commented 8 months ago

Hi! Thank you for your review and for spending your time on it.

Regarding point 1, I must admit that I didn't have enough experience with FSD when I designed this part of the application. This is my first FSD project. I agree with you that having an API in the entities layer has some benefits. The entity logic is not shared between multiple layers, and the architecture has better context boundaries. I've seen many examples with the API segment in the shared layer. The main approach is to have the ability to move the API to another project or library. In the future, I also plan to create a desktop application, and perhaps having this segment in the shared layer could be useful in that case.

Regarding your question about trying the app online, this application is designed for mobile devices. You can find the Google Play Store link or a direct link to the APK file in the given GitHub link. You can install and try it if you have an Android device. Unfortunately, I don't have an official build for iOS at the moment. If you don't have an Android device, you can try this application with an emulator.

illright commented 8 months ago

Usually it's recommended to put the abstract API client to shared/api, things like configuring the headers, JSON parsing, convenience functions for GET, POST, etc. If you want to share the API layer between the mobile app and desktop app, you don't have to keep it in shared. You can let the code take its semantic place next to the entities, but possibly consider setting up a monorepo where one package api would contain just the shared and entities and the other two packages, mobile and desktop, would consume this package and have an independent FSD root that may also include its own shared and entities, apart from consuming your api package.

Could you resolve the conflict on this branch, please? Then it would be good to merge

netlify[bot] commented 8 months ago

Deploy Preview for pr-fsd ready!

Name Link
Latest commit aa5719aaa731ba9fda9a5d65923c31c653666577
Latest deploy log https://app.netlify.com/sites/pr-fsd/deploys/652841bbe75a950008ffa150
Deploy Preview https://deploy-preview-622--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.

astsu-dev commented 8 months ago

I appreciate your suggestions, and I will keep them in mind for future reference. I resolved the conflict.