Skip to content
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

Reimplement Monitor view as React functional component with hooks #2013

Open
yurishkuro opened this issue Dec 2, 2023 · 8 comments
Open

Comments

@yurishkuro
Copy link
Member

packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx is a class based component with pretty convoluted state management. It can be simplified if implemented as a functional component with hooks.

References:

Related/similar PRs:

@yurishkuro yurishkuro changed the title Reimplement Monitor view as Reach functional component with hooks Reimplement Monitor view as React functional component with hooks Dec 2, 2023
@takuabonn
Copy link

hi @yurishkuro
I want to work in this issue. will u assign it to me?

@yurishkuro
Copy link
Member Author

Go ahead

@Sweetdevil144
Copy link

So, we're gonna flip the MonitorATMServicesViewImpl class to a functional component. Here's the game plan of how I would proceed with this issue:

  1. State with useState: For each state variable in your class component, we'll switch to using useState. It's pretty straightforward. Like, if you have a state variable graphWidth in your class, it turns into:

    const [graphWidth, setGraphWidth] = useState(300);

    Repeat this for each piece of state (serviceOpsMetrics, searchOps, etc.).

  2. Lifecycle Methods with useEffect: Your componentDidMount, componentDidUpdate, and componentWillUnmount all get replaced by useEffect. Here's an example that covers component mounting and unmounting logic:

    useEffect(() => {
        fetchServices(); // Replace with your actual data fetching or setup code
        // Any additional setup code goes here
    
        return () => {
            // This is your cleanup code, like componentWillUnmount
            // e.g., remove event listeners
        };
    }, []); // The empty array makes it run only once on mount and unmount
  3. Refactor Methods to Functions: Convert class methods into functions. For instance, if you have a method for updating dimensions in your class, it would be something like:

    const updateDimensions = () => {
        // Your logic to update dimensions
    };
  4. Handling Redux: If you're using Redux, you'll use useSelector for selecting state and useDispatch for dispatching actions. Like this:

    const services = useSelector(state => state.services);
    const dispatch = useDispatch();
    
    // When you need to dispatch an action
    const fetchMetrics = () => {
        dispatch(fetchMetricsAction());
    };
  5. Testing: After refactoring, test to ensure all functionalities work as expected. Check if state updates correctly, effects run as intended, and the component behaves as it did before.

The goal is to keep the logic intact but just translate it into the functional component style. Hope this helps you envision how the refactored component will look! Let me know if there's a specific part you want more detail on.

@EshaanAgg
Copy link
Contributor

Hi! Is this issue available to work on?

@yurishkuro
Copy link
Member Author

@EshaanAgg yes, since there is no PR attached

@RISHIKESHk07
Copy link
Contributor

@yurishkuro how to check if my code is running correct after changing the component

@RISHIKESHk07
Copy link
Contributor

@yurishkuro i did rewrite the monitor , i don 't see any error except for the below , is there anything i missed
'''
7:56:27 PM [vite] http proxy error at /api/services:
Error: connect ECONNREFUSED 127.0.0.1:16686
at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) (x2)
'''
image

@yurishkuro
Copy link
Member Author

why would making Reach changes result in any network-related errors?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants