-
Notifications
You must be signed in to change notification settings - Fork 662
Open
Labels
changelog:refactoringInternal, non-functional code improvementsInternal, non-functional code improvementsgood first issuehelp wantedplexusSpecific to packages/plexusSpecific to packages/plexus
Description
This issue is part of the larger effort to modernize the Jaeger UI codebase (Parent Issue #2610). The goal is to refactor the SvgLayer component from a React Class Component to a React Functional Component using Hooks.
Target Component: packages/plexus/src/Digraph/SvgLayer.tsx
Note: This component is part of the
plexuspackage, which is a graph visualization library used by Jaeger UI.
Acceptance Criteria
- The component is converted to a Functional Component.
-
this.stateis replaced withuseStateoruseReducer. - Lifecycle methods (componentDidMount, etc.) are replaced with
useEffect. -
memoizeOnecalls are converted touseMemowhere appropriate. - Static properties are handled appropriately (e.g., moved outside component or use module-level constants).
- All existing PropType validations are preserved.
- Performance: Wrap in
React.memoto maintainPureComponentbehavior. - Visual Verification: Attach "Before" and "After" screenshots to the PR to prove no visual regression.
- Tests: Existing unit tests must pass.
Plexus-Specific Guidance
| Pattern | Class Approach | Functional Approach |
|---|---|---|
| Instance ID | baseId = \prefix--${idCounter++}`` |
const baseId = useRef(\prefix--${idCounter++}`).current` |
| Memoization | memoizeOne(fn) |
useMemo(() => fn, [deps]) or keep memoizeOne with useRef |
| Static Props | static defaultProps = {...} |
Function parameter defaults or separate constant |
| Render Utils | this.renderUtils = {...} |
const renderUtils = useMemo(() => ({...}), [deps]) |
Migration Guide (Cheat Sheet)
| Feature | Old (Class) | New (Functional) |
|---|---|---|
| State | this.state = { isOpen: false } |
const [isOpen, setIsOpen] = useState(false); |
| Side Effects | componentDidMount() |
useEffect(() => { ... }, []) |
| Props | this.props.traceId |
props.traceId (or destructure: const { traceId } = props;) |
| Default Props | static defaultProps = { name: 'Guest' } |
function Component({ name = 'Guest' }) { ... } |
| Performance | class MyComp extends React.PureComponent |
export default React.memo(MyComp) |
| Context | static contextType = MyContext |
const val = useContext(MyContext) |
| Refs | this.myRef = React.createRef() |
const myRef = useRef(null) |
Common Pitfalls (Read Carefully!)
- Stale Closures: Be careful when using
useEffect. If you use a variable inside the effect that comes from props or state, it must be in the dependency array. If you omit it, the effect will use the old value. - Testing with Enzyme: Functional components do not have instances. Tests like
wrapper.instance().method()orwrapper.state('value')will fail. You should refactor these tests to simulate events (clicks) and check the rendered output instead. - Static Properties: Class components can have static properties attached directly. For functional components, you'll need to assign these after the function definition or use module-level constants.
Resources
- React Official Guide: Hooks at a Glance
- A Complete Guide to useEffect (Overreacted.io) - Essential reading for understanding dependency arrays.
- React Redux Hooks Documentation - Guide for replacing
connect(). - Thinking in React Hooks - Visual guide to the mental model shift.
- How to fetch data with React Hooks - Handling async operations.
Metadata
Metadata
Assignees
Labels
changelog:refactoringInternal, non-functional code improvementsInternal, non-functional code improvementsgood first issuehelp wantedplexusSpecific to packages/plexusSpecific to packages/plexus