CODEWITHSUNDEEP
javascriptreactjsreact-hooks
user15437524
user15437524

Reputation:

Can I use a loop inside a react hook?

Can i do this:

const [borderCountries, setBorderCountries] = useState([])
useEffect(() => {
    country.borders.forEach(c => {
        fetch(`https://restcountries.eu/rest/v2/alpha/${c}`)
            .then(res => res.json())
            .then(data => setBorderCountries([...borderCountries,data.name]))
    })
}, [])

Country borders is a prop passed to the component. If not what can I do?

Upvotes: 2

Views: 1377

Answers (2)

Zeeshan Shaheen
Zeeshan Shaheen

Reputation: 53

You can but it's Not a good practice.

Upvotes: -1

T.J. Crowder
T.J. Crowder

Reputation: 1074355

You can, but not quite like that, for a couple of reasons:

  1. Each fetch operation will overwrite the results of the previous one, because you're using borderCountries directly rather than using the callback version of setBorderCountries.
  2. Since the operation depends on the value of a prop, you need to list that prop in the useEffect dependencies array.

The minimal change is to use the callback version:

.then(data => setBorderCountries(borderCountries => [...borderCountries,data.name]))
//                               ^^^^^^^^^^^^^^^^^^^

...and add country.borders to the useEffect dependency array.

That will update your component's state each time a fetch completes.

Alternatively, gather up all of the changes and apply them at once:

Promise.all(
    country.borders.map(c =>
        fetch(`https://restcountries.eu/rest/v2/alpha/${c}`)
            .then(res => res.json())
            .then(data => data.name)
    })
).then(names => {
    setBorderCountries(borderCountries => [...borderCountries, ...names]);
});

Either way, a couple of notes:

  1. Your code is falling prey to a footgun in the fetch API: It only rejects its promise on network failure, not HTTP errors. Check the ok flag on the response object before calling .json() on it to see whether there was an HTTP error. More about that in my blog post here.

  2. You should handle the possibility that the fetch fails (whether a network error or HTTP error). Nothing in your code is currently handling promise rejection. At a minimum, add a .catch that reports the error.

  3. Since country.borders is a property, you may want to cancel any previous fetch operations that are still in progress, at least if the border it's fetching isn't still in the list.

Putting #1 and #2 together but leaving #3 as an exercise for the reader (not least because how/whether you handle that varies markedly depending on your use case, though for the cancellation part you'd use AbortController), if you want to update each time you get a result

const [borderCountries, setBorderCountries] = useState([]);
useEffect(() => {
    country.borders.forEach(c => {
        fetch(`https://restcountries.eu/rest/v2/alpha/${c}`)
            .then(res => {
                if (!res.ok) {
                    throw new Error(`HTTP error ${res.status}`);
                }
                return res.json();
            })
            .then(data => setBorderCountries(borderCountries => [...borderCountries, data.name]))
            //                               ^^^^^^^^^^^^^^^^^^^
            .catch(error => {
                // ...handle and/or report the error...
            });
    });
}, [country.borders]);
//  ^^^^^^^^^^^^^^^

Or for one update:

const [borderCountries, setBorderCountries] = useState([]);
useEffect(() => {
    Promise.all(
        country.borders.map(c =>
            fetch(`https://restcountries.eu/rest/v2/alpha/${c}`)
                .then(res => res.json())
                .then(data => data.name)
        })
    )
    .then(names => {
        setBorderCountries(borderCountries => [...borderCountries, ...names]);
    })
    .catch(error => {
        // ...handle and/or report the error...
    });
}, [country.borders]);
//  ^^^^^^^^^^^^^^^

Upvotes: 5

Related Questions