JS/React/fetch: not getting all data

Robert Angberg 0
Viewed 54

I'm building an app around the card game Magic, where you can paste a list of cards into a textbox and click a button to show card images. I use fetch and an external site API. Logging shows no errors, but the result is very inconsistent - sometimes only 1 card is shown, sometimes 2, 4, 7 etc... I need to always render all the data. I've tried for days with this.

const handleClick = async () => {
  textarea.split("\n").forEach(async (items) => {
    try {
      const response = await fetch(
        `https://api.scryfall.com/cards/named?exact=${encodeURIComponent(
          items
        )}&pretty=true`
      );
      console.log("response.status: ", response.status);
      if (!response.ok) {
        throw new Error(`Error! ${response.status}`);
      }
      const result = await response.json();
      console.log("result" + result);
      objects.sort(() => Math.random() - 0.5);
      objects.push(result);
    } catch (err) {
      console.log(err);
    } finally {
      setIsLoading(false);
      setScrydata(objects);
    }
  });
};

From the console.log I see everything is fetched, but sometimes in "batches", for example first 2, then 5. This is the problem I think(?) cause only 2 cards are then rendered. Is Promise.all the solution somehow? I've tried it but couldn't get it to work, I changed

const result = await response.json();

to

const result = await Promise.all([response.json()]);

but it doesn't work, the results are still in "batches".

Big thank you in advance for any help.

Edit, this is how I render the images:

{scrydata.length > 0 && (
  <img
    src={scrydata[0].image_uris?.normal}
    key={scrydata[0].id}
    alt="asdf"
  />
)}
{scrydata.length > 1 && (
  <img
    src={scrydata[0].image_uris?.normal}
    key={scrydata[0].id}
    alt="asdf"
  />
)} ... etc

Up to seven, which is the max. This shows images, but it's inconsistent; I want seven every time. Maybe this code could be my problem. Thanks again.

promise fetch reactjs javascript
1 Answers

You want to split this up into two separate stages:

  1. get all the images (with undefined or false for any failures)
  2. update the UI once you have those.

But what you've written doesn't do that, instead it does:

  1. for each card:
    1. get its image
    2. update the UI irrespective of whether that get succeeded

So rewrite your logic a little:

async function getCardImage(title = ``) {
  // do we need to do anything?
  if (title === ``) return false;

  try {
    const response = await fetch(...);
    if (!response.ok) throw new Error(`Error! ${response.status}`);
    return await response.json();
  } catch (e) {
    console.log(e);
  }

  // our function always returns "normal" data, it never exits by throwing.
  // That way, we can easily filter failures our of the Promise.all result
  return false;
}

async function fetchCardImages(titles = [], setIsLoading=()=>{}) {
  // do we need to do anything?
  if (titles.length === 0) return titles;

  setIsLoading(true);

  const results = (
    await Promise.all(titles.map(getCardImage))
  ).filter(Boolean);

  setIsLoading(false);

  return results;
};

With handleClick changed to:

const handleClick = async (_evt) => {
  const titles = textarea.split(`\n`).map(s => s.trim()).filter(Boolean);

  // do we need to do anything?
  if (titles.length === 0) return;

  const updates = await fetchCardImages(titles, setIsLoading);
  updateScryData(updates);

  // Don't update `objects` until you're in the
  // updateScryData function: your event handler
  // itself should have zero code that directly
  // manipulates any data.
  // 
  // And of course, make sure `updateScryData` checks
  // the passed argument, so that if it's an empty 
  // array, it just immediately returns because no
  // work needs to be done.
}

Although I'd recommend not calling it "handle click" because the fact that you're pointing to it from an onClick React attribute already tells people that. Give functions names based on what they do: in this case, getNewCardImages or something.

And then, finally, you render your scryData using a map:

function generateCardElement({ id, image_uris, description }) {
  const src = image_uris?.normal;
  if (!src) return;
  return <li key={id}><img src={src} alt={description}/></li>;
}

...

render() {
 return 
   ...
   <ul className="card-images">{scryData.map(generateCardElement)}</ul>
   ...
}
Mike 'Pomax' Kamermans0
Related