Fetch an array from API endpoint in async function

Question

I've been studying javascript for the love of it, I'm currently learning how to use async functions and my code actually works, but I know there is always room for improving.

I'm open to any help regarding my use of async functions and how I use the return of sizeJokesArray() in fetchJokes().

const jokeContainer = document.querySelector('.joke-container');
async function sizeJokesArray() {
 let url = 'https://api.icndb.com/jokes/count';
 let data = await (await fetch(url)).json();
 return data.value;
}
async function fetchJokes() {
 let url = `https://api.icndb.com/jokes/random/${length}`;
 let jokesData = [];
 let data = await (await fetch(url)).json();
 for (jokePosition in data.value) {
 jokesData.push(data.value[jokePosition].joke);
 }
 return jokesData;
}
sizeJokesArray().then(size => (length = size)); // return of sizeJokesArray
jokeContainer.addEventListener('click', event => {
 if (event.target.value === 'Fetch') {
 fetchJokes(length).then(jokesData => (jokesArray = jokesData)).then(jokesData => (console.log(jokesData)));
 }
});

<div class="joke-container">
 <div class="joke-text">
 <p>Fetch some jokes bro!!</p>
 </div>
 <div class="joke-controls">
 <button type="button" value="Fetch">Fetch Jokes</button>
 </div>
</div>

the existing console.log() it's not actually used in my code just used it here to log my results for the question purpose.

Answer 1

I've improved a few things to your script, making it more consistent over all:

;(function () {
 const jokeContainer = document.querySelector('.joke-container')
 async function fetchCount() {
 const url = 'https://api.icndb.com/jokes/count'
 const { value } = await (await fetch(url)).json()
 return value
 }
 async function fetchJokes(length = fetchCount()) {
 const url = `https://api.icndb.com/jokes/random/${await length}`
 const { value } = await (await fetch(url)).json()
 return value.map(({ joke }) => joke)
 }
 jokeContainer.addEventListener('click', async event => {
 if (event.target.value === 'Fetch') {
 const jokes = await fetchJokes()
 console.log(jokes)
 }
 })
}())

<div class="joke-container">
 <div class="joke-text">
 <p>Fetch some jokes bro!!</p>
 </div>
 <div class="joke-controls">
 <button type="button" value="Fetch">Fetch Jokes</button>
 </div>
</div>

• Wrapped your script in an IIFE to avoid polluting the global scope.
• Removed your bizarre usage of the global length variable, which you defined without using var, let, or const, and is considered bad practice.
• Used object destructuring to improve the readability of your code in some places.
• Declared your event listener as async in order to take advantage of await instead of using a then() chain to improve consistency and readability.
• Converted your imperative initialization of jokesData into a functional idiomatic approach using Array#map() in order to further improve readability.

• ricardoNava: is this const { value } = await (await fetch(url)).json() object destructuring or what is this doing? it blew my mind!
• Patrick Roberts: Yes, it's the same as const value = (await (await fetch(url)).json()).value
• ricardoNava: Learned more from your answer then like the past month studying and reading about .then(), await, async and such... Thanks now ill read about the last point you mentioned.
• Patrick Roberts: @ricardoNava it's called the destructuring assignment, but it's also used in the argument of the Array#map() callback in this answer as well.

Answer 2

Here you can improve.

sizeJokesArray().then(size => (length = size)); // return of sizeJokesArray
jokeContainer.addEventListener('click', event => {
 if (event.target.value === 'Fetch') {
 fetchJokes(length).then(jokesData => (jokesArray = jokesData)).then(jokesData => (console.log(jokesData)));
 }
});

You can change it to

let jokes = [];
jokeContainer.addEventListener('click', event => {
 if (event.target.value === 'Fetch') {
 try {
 let length = await sizeJokesArray(); // You dont get ".then" here where you were assigning length variable, if you are using async/await. Use it properly, there is no use of an async function if you are calling it in promise fashion.
 jokes = await fetchJokes(length) // Isn't it easy and clean to read that jokes are meant to be fetch and we are waiting for it.
 console.log(jokes); 
 } catch (err) {
 // Handle API error here
 }
 }
});

When you use async/await, Use it properly everywhere. instead of calling then() you should be awaiting it. also handle error via try/catch

• Vogel612: so ... why is this an improvement? Elaborate just a tad here ;)
• ricardoNava: Why would I want to have my variables in the global scope?
• I wrestled a bear once.: and, using let in the global scope is kinda pointless
• Muhammad Faizan: You dont want to use var because its more pointless. I am assuming that this function is within some module and not global. also if you use const you cant change its reference, thus you have to stick with var in this scenario
• Patrick Roberts: @Iwrestledabearonce using let in global scope is not pointless. Unlike the respective var declaration, the variable name is not accessible on the window object. So given a top-level let foo = 'bar';, window.foo does not exist.