-
Notifications
You must be signed in to change notification settings - Fork 311
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
Remove all class components and replace them functional components using hooks #6376
Comments
Once we have all class components removed, we'll also be able to refactor our redux-actions to get rid of |
As discussed offline, @ryaplots will take a crack at this this milestone to see how far we can get. |
As of today, the remaining class components are:
Via All HOCs should just be removed if they're not used anymore. Otherwise a hook version should be written for them so usage can be replaced properly. |
We're also quite far already with removing all Last standing are:
Once we've refactored these, we can remove |
We also have some functional components that still use
|
Rolling item in progress. Moved to |
Referencing this comment. |
Closing this now as completed. The remaining two components can be refactored later. What's important now is that the vast majority of components are refactored and we can now look into some more general restructuring of our codebase such as refactoring fetching and error logic which is now done completely based on promisified dispatches rather than store artifacts. Thanks @ryaplots and @PavelJankoski for the hard work on this. This made our codebase a whole lot more future-proof! |
Summary
To follow the latest best practices and simplify implementations, we should work to replace any React class component with a functional component. We already converted many in the last years and I replaced many more as part of #6327 but there is still a substantial amount left.
Current Situation
Many older components still use the class-based approach, which is becoming increasingly cumbersome as it often causes incompatibilities with upgraded dependencies, such as react-router v6 but also overall constitutes a code smell. Latest react versions clearly state functional components as the preferred modus.
Why do we need this? Who uses it, and when?
We need this to address technical debt and to make our codebase more future-proof. It's also annoying to have to convert classes on the spot while working on other features, as it takes additional time, poses potential error risks and dilutes the diff which makes reviewing harder.
Proposed Implementation
Let's make sure we do this in a consistent manner. Here's are the guidelines I followed when converting components:
withRequest()
-HOC to fetch initial data should be split into two functional components (outer and inner, arrow functions, the outer should keep the original name), so that<RequireRequest requestAction={exampleActionCreator(param1, param2)} />
can be run in the outer function to ensure that all required data will be available in the inner class when usinguseSelect
.connect()
-HOC should be refactored to useuseSelect()
instead (in the inner component)withBreadcrumbs()
-HOC should useuseBreadcrumbs('id.path', <Breadcrumbs … />)
-hook instead (same signature aswithBreadcrumbs
)withFeatureRequirement()
-HOC for feature flags should use<Require featureCheck={mayDoAction} otherwise={{ redirect: '/example-route' }} />
insteaduseCallback()
Contributing
Code of Conduct
The text was updated successfully, but these errors were encountered: