Roast my JavaScript please

Code Feedback
6

Turning async off makes the request synchronous, so the requests will stop the rest of your app from working until they are complete. JavaScript is single threaded, so we can only ever do one thing at a time in an app. However, the browser’s APIs allow us to send a task off to be completed by another thread, freeing the app up to do other things, like render the page or respond to user input. This is what happens when we make AJAX requests - we give the browser a url to contact and a callback function to run when the response comes back, but JS itself doesn’t make the request. This is asynchronous JavaScript in a nutshell, and by turning async off you’re forcing your app to wait on the slowest part of your code. Ironically, this setting isn’t even valid because you set the datatype to jsonp, which disables synchronous requests. So, might as well leave it at default.

Setting the datatype to ‘jsonp’ isn’t necessary because the API you’re calling supports CORS. JSONP is a hack that web devs have had to use in order to get around the same origin policy (SOP). This is all really confusing, and I wouldn’t be doing you any favors trying to explain it here. The short story is that SOP is a really good thing but it has made life difficult for developers needing to get data from a remote source. JSONP was only needed because we didn’t have a formal way for data servers to decide who can get information remotely. Now that we have CORS, JSONP will go the way of the non-avian dinosaurs. Your app works after deleting both these options, so no sense in having them specified.

Imagine that more than one function depends on your global variables. Since all the functions are accessing the same data, if any one of them changes that data, then they’re all going to be affected. Now imagine that you have asynchronous network calls which write this data - you don’t know when these network calls will be finished, so they could write to your variables when you’re not expecting them to, way before you want them to, or not at all. Managing global variables can be like herding cats. In your case, you made good use of jQuery’s AJAX events and your app only runs when all of the network calls have finished. There are a few problems with this approach in bigger apps, however.

  • You’d have to write a lot of code to handle errors. The API you’re using is simple and not error prone, but in bigger apps we have to worry about more than just the happy path.

  • Relying on global AJAX events can also be a lot slower than processing each piece of data as it comes in. Your app only has to wait on a few calls to finish, and they’ll all be successful in a reasonable amount of time, but modern apps can make hundreds of requests. Think about Netflix - each movie that you see is the result of a request (I think each row of movies is a single request, but not entirely sure). If Netflix waited until all of the data had come in before rendering, the app would seem a lot slower.

  • Your app makes one request when it’s loaded, but we often have to make multiple requests throughout the app’s life. If you wanted to allow users to search for Twitch users and add them to a list, working around the global AJAX events becomes a much bigger hassle than it’s worth.

The idea is not to store the data, but to render it as it comes in. There’s really no sense in holding onto it. Check out my Codepen I linked to. On line 14, I render each individual user:

$(".users").append(userCardTemplate(user));

userCardTemplate is a Handlebars function I created. All it does is take a piece of data and apply it to a template. It’s really no different from what you’re doing on line 64 of your own app, just building markup in a string and then appending it to another element. @blogscot’s got a great example in his post using the template literals, but imagine that string being returned from a function:

function streamingUserTemplate(user) {
 return `<div class="resultBox">
             <h1>${user.streamerName}</h1>
             <p>online playing: ${user.streamStatus}</p>
             <a target="_blank" href="${userl.streamerLink}">
               <button class="onlineBtn">Stream</button>
             </a>
           </div>`
}

// now look at how easy it is to render each user
for(let i=0;i<streamArray.length;i++){
    var url = `https://wind-bow.glitch.me/twitch-api/streams/${streamArray[i]}`;

    $.getJSON(url, function(response) {
        if(status !== null) {
           $resultBoxMain.append(streamingUserTemplate(response));
        } else {
           $resultBoxMain.append(offlineUserTemplate(response));
        }
    });    
}

Now you’re not herding cats, you can handle errors more easily, and your app will be more performant.

You can also kick back and do some watching:

1 Like