-
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 wanted
Description
This issue is part of the larger effort to modernize the Jaeger UI codebase (Parent Issue #2610). The goal is to refactor the TraceSpanView component from a React Class Component to a React Functional Component using Hooks.
Target Component: packages/jaeger-ui/src/components/TracePage/TraceSpanView/index.tsx
Acceptance Criteria
- The component is converted to a Functional Component.
-
this.stateis replaced withuseStateoruseReducer. - Lifecycle methods (componentDidMount, etc.) are replaced with
useEffect. - If Redux is used, the
connectHOC is replaced withuseSelectoranduseDispatchhooks (optional but preferred). - All existing PropType validations are preserved.
- Performance: If the original component was a
PureComponent, the new one is wrapped inReact.memo. - Visual Verification: Attach "Before" and "After" screenshots to the PR to prove no visual regression.
- Tests: Existing unit tests must pass.
- Note: If tests rely on Enzyme's
state()orinstance(), they must be refactored to check the DOM or Props instead.
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.
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 wanted