<Mohammed> - React - Week2

[mo92othman]

No description provided.

[mo92othman]

Deployed app:
https://react-week2-shop.netlify.app/

[durw4rd]

Great job this week, Mo. You are almost there but before approving, I would like you to add the handling of the loading and error states for the components that depend on fetching data via API requests.
I've given you an example of a custom hook that you could use for this but if you don't like that approach, feel free to go about this any way you'd like. If you would like to use the custom hook but can't quite make it work, feel free to ping me on Slack!

[durw4rd]

Creating custom functions for fetching data from an API is a great idea! To bring this approach one step further, you could turn this into a custom React hook.
Creating a custom hook would also allow you to centrally manage the loading and error states - it's a requirement for this week and I noticed that for some requests (request to fetch categories and all/filtered products), the loading/error states are not managed.
Here is an example of a useFetch custom hook to which you pass a URL that it should fetch from:

import { useState, useEffect } from 'react';

const useFetch = (url) => {
    const [data, setData] = useState(null);
    const [loading, setLoading] = useState(true);
    const [error, setError] = useState(null);

    useEffect(() => {
        setLoading(true);
        setError(null);
        const fetchData = async () => {
            try {
                const response = await fetch(url);
                if (!response.ok) {
                    throw new Error('Failed to fetch data');
                }
                const jsonData = await response.json();
                setData(jsonData);
            } catch (error) {
                setError(error);
            } finally {
                setLoading(false);
            }
        };

        fetchData();
    }, [url]);

    return { data, loading, error };
};

export default useFetch;

The above hook is a simplified version of the example described in this article. In the article, you can also see how the hook's return values are used inside the return statements of the components that use it.

[durw4rd]

You are not handling the loading and error states here. Would you know how to add this?
One option is to use the custom hook example I mentioned in the comment for the dataServices file instead of the generic useEffect hook and then render the content according to what's returned by the hook - something like:

[...]
const { data, loading, error } = useFetch('https://fakestoreapi.com/products/categories');
[...]
<ul className="list">
        { loading ? 'Loading...' : error?.message || data.map((category) => (
[...]

But feel free to use any approach you like as long as you add the loading and error states! :)

[mo92othman]

Great! Thanks a lot

[durw4rd]

Similar feedback as for the CategoryList component - can you add the handling of loading and error states for this component?

[durw4rd]

Very nice - I like the use of the useParams hook!

[durw4rd]

The handling of loading/error states is better here than for the other API requests but the error state is still quite hidden from the user unless they look into the browser console. Can you improve the handling of the two states (while ideally making it consistent across the different API requests)?

[mo92othman]

Thank you so much for your great feedback! I see now how the loading/error states weren't managed well in my previous code.
I used the way that you recommended to me, to create a custom hook that returns data, loading, and error.

What I did slightly differently is that I didn't use that hook directly in my components, instead, I used it in functions that use the custom hook with different scenarios or parameters, and then I used those functions inside my components, do you think this is a good approach?

Thanks again for your feedback.

[mo92othman]

I have a question here, this custom hook will return { data, loading, error };
And then when we use it in a component, we let the component knows that there is an error in case there is an error, but then we display something for the end-user in the front-end in h or img elements.

In other words, we pass that to the component and then we deal with whatever we want to display in the component itself.

Is this a good approach or should we handle / setError here something differently?

[durw4rd]

It's a good question. I would say it boils down to how standardized and/or complex the error handling is across the different components. In the relatively straightforward scenarios - such as the webshop we're building here - I think it's perfectly fine to leave the handling of return values as part of the component rendering logic.
If things get more complex, it could make sense to take parts of the conditional logic out of the return statement to keep things easy to follow.

[mo92othman]

One more question here, as I mentioned in my previous comment, I didn't use the useFetch custom hook directly in my components, instead I used it in this file, where I use those functions in my components.

I am running the project and it is working perfectly.
But then in this file I get a red line under useFetch and I get this msg. I didn't get exactly what is wrong here, should it be structured in another way? or no worries?

[durw4rd]

What you're doing here is essentially defining three additional custom React hooks based on the original useFetch custom hook. The naming convention in React is that any hook should start with the prefix use - if you rename the functions by adding 'use' at the beginning of their names ('useFetchCategories', 'useFetchProducts', ...) the warning should go away. More on this in the purple 'deep dive' section here.
I would also add, though, that having the chain of two custom hooks to fetch data from the different API endpoints seems unnecessarily complex (and therefore harder to debug, especially for someone who didn't write the code), and wouldn't scale too well - imagine your app would be calling 10 or 15 different endpoints instead of just three. It's not necessary but my (soft) recommendation would be to have a single custom hook for fetching the data and handling the rest of the logic within the individual React components.

[durw4rd]

Thanks for implementing the changes, Mo. For next week's homework, you will need to resolve the linter warning around calling React hook in a standard JS function as discussed in the comment above but for now, you're all set - good job!