[Feature]: Refactor class components to functional components #2610
Labels
Comments
|
don't we have already an issue for that? |
|
hmm as i said in last section, i found those 2, which still doesn't give us the comprehensive list. BUT if we got that already and somehow i missed it then we can just close this down and feel free to copy lists from here to there. |
|
ok, #2610 is only a part of this, so ok to have a new tracker ticket. |
This was referenced Jan 24, 2025
|
Hey @yurishkuro , I've created a PR for migrating DetailsCard from class based to function based component - #2637, could you please have a look at it? I'll be pushing the tests soon 👍 |
|
Hi @yurishkuro , I would like to work on some of the items from this list. Will PR on this issue get merged? |
yes |
4 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 26, 2025
…2638) ## Which problem is this PR solving? Converted EmphasizedNode in #2610 ## Description of the changes - Refactored class components to functional components for EmphasizedNode.tsx for Issue #2610. - Renamed the Props to EmphasizedNodeProps ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Aditya Sahu <[email protected]> Signed-off-by: Aditya Sahu <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Requirement
Jaeger UI still have a number of core components that are React class-based components. This prevents the use of hooks in them, and newest DX on them.
Refactor all the possible components in Jaeger UI that still are class-based components. Convert them to functional components, whenever possible.
Problem & Motivation
The idea is also while moving to them, use hooks to abstract the functionality present in Redux and in data fetching so in a near future we can easily switch using the hooks to use other props provider.
Proposal & Implementation
Refactor should be done carefully and providing tests where they do not exist. The lifecycle of the components should be preserved while watching out for edge cases and UX problems.
Deliverables
IMO It should be delivered all at once, as a major breaking change. Ideally, we won't change the contract of any of the components, so no new props passed down, no return anything to the tree that we were not returning before, unless it's justified.
If the contract has to be broken, then it must be properly documented and exposed in the upgrade guide.
Here there is a preliminary list, subject to change:
Complex / Optional:
(list of complex or optional ones, so move those issues here from above)
Risks
The lifecycle of a component could easily be changed if the refactor is not done carefully and giving attention to details, that's why we should provide acceptance tests and heavy QA should be done while doing it.
There are several issues/PRs that exist (e.g., #2588, #2013), but I believe it would be most beneficial to maintain/track them properly by having a single, comprehensive list.
Open questions
No response
The text was updated successfully, but these errors were encountered: