2
\$\begingroup\$

Today I was trying to load navigation bar from another HTML page. Here is code, that I am using. What do you think about it? Is there any other way to do it?

var x = document.getElementsByClassName("PageLoader"); var url = ""; for (var i = 0;i<x.length;i++){ url = x[i].dataset.url; var xmlHttp = new XMLHttpRequest(); xmlHttp.open( "GET", url, false ); xmlHttp.send( null ); x[i].innerHTML = xmlHttp.responseText; } 
\$\endgroup\$
1
  • \$\begingroup\$When you ask "Is there any other way to do it?": This depends strongly on why you did it like this in the first place. Most importantly: Why are you using JavaScript for this instead of using server-side includes?\$\endgroup\$
    – RoToRa
    CommentedFeb 14, 2018 at 8:41

2 Answers 2

4
\$\begingroup\$

I have some issues with your code and possible alternatives for implementation.

Usage of var

In Javascript using var instead of let or const are somewhat discouraged (in the new era of ES6). There is some value in block scope for loops using var but you can certainly always use ES6' let instead. Since you are not reassigning xmlHttp you might as well const that (same goes for x and let for url).

Use fetch instead of XMLHttpRequest

Fetch comes with a promise-ready easier API for handling requests. CanIUse reports that it's almost supported on every new Browser as well. A polyfill should be enough to support the rest of the browsers out there. It also helps in readability and simplicity.

In your case, getting HTML out of a requests is a simple as:

fetch(...) .then(function (response) { // Status 200? Type text/html? Might aswell check that here. return response.text(); }) .then(function (body) { // Insert into dom here... console.debug(body); }) 

Error handling thanks to fetch

What happens if your URL is faulty, temporarily unavailable and some other error? If you need error handling then just add a catch below then:

.catch(e){ // Display message, console silenty console.debug(e); } 

Using external libraries is usually not necessary as ES6 brings the most bang for the buck anyway.

\$\endgroup\$
    0
    \$\begingroup\$

    What do you think about it?

    Passing false as the 3rd parameter (i.e. async) to .open() is somewhat questionable. Is there a reason that is desired? Perhaps the goal was to have the requests run in series. Otherwise, if having them run in parallel is acceptable, then the load time of all requests could be shorter. Of course, one would have to wait for the response to be finished before utilizing the responseText, and ensure that the value of i is maintained properly. One way to do that is to use Function.bind() to make a partially applied function. Expand the code snippet below and run it for a demonstration.

    var x = document.getElementsByClassName("PageLoader"); var url = ""; for (var i = 0; i < x.length; i++) { url = x[i].dataset.url; var xmlHttp = new XMLHttpRequest(); xmlHttp.open("GET", url); xmlHttp.send(null); xmlHttp.onload = function(xmlHttp, i) { if (xmlHttp.readyState === XMLHttpRequest.DONE) { if (xmlHttp.status === 200) { console.log('readyState == DONE, status == 200; i: ', i); x[i].innerHTML = xmlHttp.responseText } else { console.log('readyState == DONE, status: ', xmlHttp.status); } } //bind context to a null object, pass the current xmlHttp instance and i to the function }.bind(null, xmlHttp, i); }
    .cell { display: table-cell; max-width: 200px; word-wrap: break-word; border: 2px solid #f00; padding: 8px; margin: 5px; }
    <div class="cell">Badges: <div data-url="https://api.stackexchange.com/2.2/badges?order=desc&sort=rank&site=stackoverflow&pagesize=1" class="PageLoader"></div> </div> <div class="cell">Tags: <div data-url="https://api.stackexchange.com/2.2/tags?order=desc&sort=popular&site=stackoverflow&pagesize=1" class="PageLoader"></div> </div>

    Is there any other way to do it?

    Yes, in addition to the code sample above, you could also utilize one of many libraries that abstract the XMLHttpRequest functionality into wrappers (A.K.A. AJAX wrappers). Many of those wrappers utilize promises. The XMLHttpRequest code could be abstracted into a wrapper using Promises (see this example). For a comparison of many popular libraries, look at this page.

    One example is the fetch API. Be aware though browser compatibility could be an issue (e.g. if support for older browsers like IE is needed).

    \$\endgroup\$