-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
const StateHistoryCount = () => ( | ||
<> | ||
<Count /> | ||
<SetCountButton /> | ||
<UndoCountButton /> | ||
<RedoCountButton /> | ||
<ResetCountButton /> | ||
</> | ||
); |
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.
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
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.
left some small comments :)
Co-authored-by: Yuki Noda <[email protected]>
Co-authored-by: Yuki Noda <[email protected]>
Co-authored-by: Yuki Noda <[email protected]>
@yukinoda Thanks for your review! I have committed some of your suggestions! 🙏🏼 |
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.
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!!
@n-igarashi @yukinoda It was decided to merge this PR at the weekly meeting 🚀 Thanks for the review! |
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)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.
Benefits of using Storybook with component-driven develop:
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