Skip to content
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

Refactoring for separation of concerns #26

Merged
merged 29 commits into from Mar 3, 2022
Merged

Conversation

jinmayamashita
Copy link
Collaborator

@jinmayamashita jinmayamashita commented Dec 13, 2021

This PR is Experiments and proposals for the separation of concerns.
If you are running a boilerplate which you want to run on localhost, use yarn dev

What I did:

Separation of states

I think data separation should be made between UI state and server state data because their characteristics differ greatly.

I tried RTK Query and React Query both of which take over cache management. #19
They are basically the same approach, also RTK Query don't need to depend on redux.

But React Query seems it more intuitive than RTK Query with just a few lines of code.

https://github.com/monstar-lab-oss/reactjs-boilerplate/blob/7172283a977cbc390e343c01c7e72b0d78cf7c37/src/pages/Profile.tsx#L15-L16

UI state

However, RTK Query/React Query is not a replacement for local/client state.
Thus, we can make two options for the UI state. (like modal isOpen state)

  • use existing state management libraries (e.g. Redux, MobX, Zustand)
  • we just need global split context for the preventing re-renders.

I recommand using global state because the share of UI state and the code needed to manage it is very small for most applications. Also, we can prevent preventing re-renders with like a constate.

☝🏼 Check the console to see if it's re-rendered.

Modular and scalable components

I understand there is not much point in talking about reusability when there is zero or one client for a given component.
#3 (reply in thread)

But let's rethink this from a component-driven perspective with Storybook.

It can be used locally, or deployed automatically so designers, PMs, clients and other stakeholders can access it.
https://monstarlab-jp.atlassian.net/wiki/spaces/CD/pages/372899912/Storybook by @yannde

Benefits of using Storybook with component-driven develop:

  • Looking it up in an explorer is a much easier way for a colleague to review a new or changed component
  • UI testing should be quick and easy. Pinpoint bugs down to the detail by testing at the component level. It’s less work and more precise than testing screens.
  • Storybook erases this problem by giving designers and developers an interpreter that makes communication easier.

And that’s why we'll want to factor out smaller components where possible, following the principle of separation of concerns.

https://github.com/monstar-lab-oss/reactjs-boilerplate/blob/e8b699a755ab6bedfb20c7348e9720fd71dfb8a0/src/components/atoms/Button.test.tsx#L3-L14

Separation of concern with hooks

Components can be made more testable by making sure that the hooks that manage the business logic are passed as props. (e.g useAuth)

https://github.com/monstar-lab-oss/reactjs-boilerplate/blob/dc955878a8c77eddab73c7e6b249659f3078796b/src/components/molecules/Header.tsx#L5-L11

https://github.com/monstar-lab-oss/reactjs-boilerplate/blob/dc955878a8c77eddab73c7e6b249659f3078796b/src/components/molecules/Header.stories.tsx#L15-L22

More Experiments:

TODO

  • RestrictAccess

@jinmayamashita jinmayamashita changed the title [WIP] Refactoring for separation of concerns Refactoring for separation of concerns Jan 17, 2022
Comment on lines +48 to +56
const StateHistoryCount = () => (
<>
<Count />
<SetCountButton />
<UndoCountButton />
<RedoCountButton />
<ResetCountButton />
</>
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying out a counter with redo/undo functionality in zustand.
re-render appears to be prevented 👀

Kapture.2022-03-02.at.11.35.28.mp4

Copy link
Contributor

@yukinoda yukinoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some small comments :)

__mocks__/api/login.ts Outdated Show resolved Hide resolved
src/pages/NotFound.tsx Outdated Show resolved Hide resolved
src/components/layouts/BlankLayout.tsx Show resolved Hide resolved
src/routes.tsx Outdated Show resolved Hide resolved
src/types/path.ts Show resolved Hide resolved
Co-authored-by: Yuki Noda <16083322+yukinoda@users.noreply.github.com>
jinmayamashita and others added 2 commits March 2, 2022 14:45
Co-authored-by: Yuki Noda <16083322+yukinoda@users.noreply.github.com>
Co-authored-by: Yuki Noda <16083322+yukinoda@users.noreply.github.com>
@jinmayamashita
Copy link
Collaborator Author

@yukinoda Thanks for your review! I have committed some of your suggestions! 🙏🏼

Copy link
Contributor

@yukinoda yukinoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will leave the rest of the review to the one assigned in the weekly meeting.
Great to see that the boilerplate OSS is active! Thanks for your contribution!!

README.md Show resolved Hide resolved
@jinmayamashita
Copy link
Collaborator Author

@n-igarashi @yukinoda
FYI @mvn-luanvu-hn @yannde @mvn-binhnguyen-hn @don-wang

It was decided to merge this PR at the weekly meeting 🚀 Thanks for the review!

@jinmayamashita jinmayamashita merged commit c3136e0 into main Mar 3, 2022
@jinmayamashita jinmayamashita deleted the try-simply-folders branch March 3, 2022 08:24
@jinmayamashita jinmayamashita self-assigned this Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants