1

I am attempting to loop through images in a block of HTML and get the native width of each image. I have the DOM build perfectly and I'm using image-size module on NPM to retrieve the image widths.

The problem is it takes time to fetch the images and get their width, so the code continues processing before I've got the widths back. As a result I cannot adjust the widths in the HTML block because the functions run and complete before getting the first image back.

Anyway to stop the code from processing until this GET request finishes? I don't want the For-Loop to continue till the image completes.

  var elem_tags = doc.getElementsByTagName("img");
  var elem_tags_length = elem_tags.length;

  for (var i=0; i < elem_tags_length; i++) {
    var imgUrl = options.elem_tags[i].getAttribute('src');
    http.get(imgUrl, function (response) {

      // My Code To Manipulate <img> tags
      var chunks = [];
      response.on('data', function (chunk) {
        chunks.push(chunk);
      }).on('end', function() {
        var buffer = Buffer.concat(chunks);
        console.log(imgSize(buffer).width); // imgSize is a module from NPM. Disregard for our loop purposes. 
      });


    });

  }

My Full Code For Reference:

var url = require('/usr/lib/node_modules/url');
var http = require('http');
var https = require('https');
var jsdom = require("/usr/lib/node_modules/jsdom").jsdom;
var imgSize = require('/usr/lib/node_modules/image-size/');

var myhtml = '<img src="http://xdesktopwallpapers.com/wp-content/uploads/2011/11-1/Searching-For-Something.jpg" /> <div style="width:500px;border:2px;" id="mytestdiv"><p style="margin:40px;">Harry Potter <img src="https://sites01.lsu.edu/wp/lsupd/files/2011/01/poster1.jpg" style="width:900px" /> and <img width="999" src="http://xdesktopwallpapers.com/wp-content/uploads/2011/11-1/Searching-For-Something.jpg" /> and <img style="width:190px" width="190" src="http://xdesktopwallpapers.com/wp-content/uploads/2011/11-1/Searching-For-Something.jpg" /></p></div>';

function getImage(imgUrl) {
  console.log('image loop');
    return new Promise(function(resolve, reject) {
        http.get(imgUrl, function(err, result) {
            if (err) return reject(err);
            return resolve(result);
        });
    });
}

var doc = jsdom(myhtml);
var doc = doc.parentWindow.document;    

var elem_tags = doc.getElementsByTagName("img");
var elem_tags_length = elem_tags.length;
var promises = [];

for (var i=0; i < elem_tags_length; i++) {

    var imgUrl = elem_tags[i].getAttribute('src');

    var promise = getImage(imgUrl).then(function(response) {
        // My Code To Manipulate <img> tags .... return promise if async
    });

    promises.push(promise);

}

Promise.all(promises).then(function() {
    console.log('done');
});
DigitalMC
  • 805
  • 2
  • 16
  • 31
  • As it's not clear what the `getProtocolVar.get()` method returns, i.e. promise or not, a simple counter inside the callback, and checking for `counter === elem_tags_length` to know that all images are loaded is an option – adeneo Apr 28 '16 at 20:09
  • 3
    Synchronous code is not the way to code nodejs, learn to use callbacks and promises. – Ruan Mendes Apr 28 '16 at 20:12
  • 1
    You can use `async.js` to manage callbacks and chaining. https://github.com/caolan/async – Phagun Baya Apr 28 '16 at 20:13
  • 1
    Use async, the eachSeries method. – yBrodsky Apr 28 '16 at 20:15
  • @adeneo I'm sorry I forgot to update that code. It is is the `http` function. Any Ideas how to use promises. in this circumstance. I'm pretty new to node. – DigitalMC Apr 28 '16 at 20:15
  • "[Asynchronous for cycle in JavaScript](http://stackoverflow.com/questions/4288759/asynchronous-for-cycle-in-javascript)" or "[Correct way to write loops for promise](http://stackoverflow.com/questions/24660096/correct-way-to-write-loops-for-promise)" may also help. – Jonathan Lonowski Apr 28 '16 at 20:19
  • @user1655756 I tried the Async plugin but couldn't figure it out at all. I spent a couple hours with it to no avail. It wasn't stopping the rest of the code from finishing. – DigitalMC Apr 29 '16 at 16:45

2 Answers2

2

Create a function that returns a promise, or promisify the http class with middleware

function getImage(imgUrl) {
    return new Promise(function(resolve, reject) {
        http.get(imgUrl, function(response) {
            var image = '';

            response.on('data', function(data) {
                image += data;
            });

            response.on('end', function() {
                return resolve(image);
            });

            response.on('error', reject);
        });
    });
}

Then iterate and store the promises

var elem_tags = doc.getElementsByTagName("img");
var elem_tags_length = elem_tags.length;
var promises = [];

for (var i=0; i < elem_tags_length; i++) {

    var imgUrl = options.elem_tags[i].getAttribute('src');

    var promise = getImage(imgUrl).then(function(response) {
        // My Code To Manipulate <img> tags .... return promise if async
    });

    promises.push(promise);

}

Promise.all(promises).then(function() {
    // all done
});
adeneo
  • 312,895
  • 29
  • 395
  • 388
  • The only detail is that you have to wait for the `data` and `end` events. See https://davidwalsh.name/nodejs-http-request – Ruan Mendes Apr 28 '16 at 20:19
  • Thanks for the idea! I'm trying it now. @JuanMendes I use `data` and `end` but this doesn't stop the code outside this GET from continuing. – DigitalMC Apr 28 '16 at 20:28
  • The only problem here is I need to modify the image tags. So once the `for` loop continues I can no longer go back and update that image since it finishes and outputs the 'modified' HTML (which in this case has not changed). – DigitalMC Apr 28 '16 at 20:42
  • 1
    @DigitalMC As adeneo pointed out, create a `Promise` where the comments indicate to return a `Promise`, don't resolve that promise until you get the `end` event – Ruan Mendes Apr 28 '16 at 20:42
  • @JuanMendes - ah, that's right, I'm used to the Request middleware, where you don't have use the specific events etc. so I just copy pasted the OP's code, but one would have to wait for the `end` event before continuing. – adeneo Apr 28 '16 at 21:00
  • @JuanMendes - also, I believe later versions of `http` call `end` automatically -> https://nodejs.org/api/http.html#http_http_get_options_callback – adeneo Apr 28 '16 at 21:02
  • I still can't get this to work. I've added my full code above. I get 4 logs for `image loop` which is correct but I never get the `done` console.log. The script finishes processing before the AJAX calls. I'm starting to think this is impossible. – DigitalMC Apr 28 '16 at 23:12
  • I can't really test it, the best I can do is mock up the functions and images and test it in jsFiddle, and it seems to work there -> **https://jsfiddle.net/4puw33bn/1/** – adeneo Apr 29 '16 at 08:56
  • Thanks for that. Based on your fiddle I figured something out. You setting `var http ...` instead of using the "default" http variable made it work without downloading images. So then i tried using real images and found adding the `response.on(...).on('end'...)` code also caused it to never evaluate the `Promise.all` code. I'll do a bunch more testing and see fi I can't figure it out. – DigitalMC Apr 29 '16 at 16:44
  • Still getting the loop working properly, but never hitting the `Promise.all`. So weird. – DigitalMC Apr 29 '16 at 19:37
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/110673/discussion-between-digitalmc-and-adeneo). – DigitalMC Apr 29 '16 at 19:58
  • 1
    The only reason I can think of for the `Promise.all` not to fire, is if all the promises aren't resolved, you must be missing something, or one or more of the images fail to do what they are supposed to do etc. – adeneo Apr 29 '16 at 21:24
1

Using async eachSeries method

async.eachSeries(yourArray, function(item, cb) {
   http.get(imgUrl, function (result) {
    //Do whatever you want with result
    //call the cb function of the async to continue the loop
    cb();

  })
}, function(){
  //Once your loop is finished, this function will be called
})
yBrodsky
  • 4,981
  • 3
  • 20
  • 31
  • Cool, I'll give this guys a shot. So I put `http.get(imgUrl, function (response)...` inside of the `yourGetFunction` function? – DigitalMC Apr 29 '16 at 17:59
  • Gotcha, now here is where I get confused. What is `yourArray`? How do I store a list of functions in an array or am I totally backwards? – DigitalMC Apr 29 '16 at 19:43
  • yourArray is whatever array you have. In your case, you have an array of elements. 'elem_tags'. Check out this example: https://jsfiddle.net/yLrnrpsc/ – yBrodsky Apr 29 '16 at 20:47