user14787312
user14787312

Reputation: 33

useEffect doesn't wait Promise.all?

I want to fetch data for every object in array passed as props (rootComments) and add the data to object properties:

useEffect(() => {
  rootComments.map(rootComment => {
    if (rootComment.kids) {
      rootComment.kidsText = []
      fetchKidsText(rootComment.kids)
    }
  })
  console.log(rootComments)
  rootComments.map(comment => console.log(Object.values(comment)))
}, [rootComments])

const fetchKidsText = async (ids) => {
  await Promise.all(
    ids.map(id => fetch(`https://hacker-news.firebaseio.com/v0/item/${id}.json?print=pretty`).then(response => response.json()).then(result => {
      rootComments.map(comment => {
        if (comment.id === result.parent) {
          comment.kidsText = comment.kidsText.concat(result.text)
        }
      })
    }))
  );
}

It seems like it doesn't work, I can't render rootComments.map(comment => comment.kidsText) and I can't log it as rootComments.map(comment => console.log(Object.values(comment))) (acts like there's no new kidsText property) or as rootComments.map(comment => console.log(comment.kidsText)) (returns empty array). But. When I log console.log(rootComments) it returns array of objects with new kidsText property.

My first thought was that useEffect() doesn't wait for rootComments to fetch inside another component but it doesn't seem to be a problem so maybe it's something inside fetchKidsText() or with the way it interacts with useEffect()?

Upvotes: 3

Views: 886

Answers (1)

kigiri
kigiri

Reputation: 3368

A few things are wrong with this code:

  • You mutate your rootComments object without using setState (react can't update from it)
  • You use map for iteration but discard the result
  • Mix of .then in async / await function
  • You don't return the value of Promise.all and instead await it

.map is not a for loop, either use .forEach or a for..of loop

As for your actual question, my first point is probably the main issue, show more code so we can confirm

edit suggested alternative code:

useEffect(() => {
  // get all the kids id, this create a single big array with every kids:
  const allKids = rootComments.flatMap(rootComment => rootComment.kids || [])

  // use a controller too cancel fetch request if component is unmounted
  // otherwhise, trying to setState if the component is unmounted will
  // mess react up:
  const controller = new AbortController()

  // we fetch all the kids at once instead of doing this in a map like before
  fetchKidsText(allKids, controller.signal)
    .then(setRootComment) // note that i'm using `.then` here,
    // because we can't await in hooks we must return a callback or undefined.
    .catch(err => {
      if (err.name === 'AbortError') return // we can safely ignore abort error
      // ? handle error here
    })
  return () => controller.abort()
}, [rootComments])

const fetchKidsText = async (ids, signal) => {
  // first we call the api and get everything a big result
  const results = await Promise.all(ids.map(async id => {
    const url = `https://hacker-news.firebaseio.com/v0/item/${id}.json?print=pretty`
    const res = await fetch(url, { signal })
    return res.json()
  }))

  // now what you were trying to do was merge every children text comment
  // since we got them all, just one filter should work:
  return rootComments.map(comment => {
    const children = results.filter(r => r.parent === comment.id)
    const kidsText = children.map(r => r.text)
    return { ...comments, kidsText }
  })
}

Don't use this code as is, I didn't test it's just to explain how I would have try to solve your problem with comments to explain my resoning.

Good luck !

Upvotes: 6

Related Questions