1

I have an onload function within a for statement that is producing unexpected results.

The for loop loops through an array of images. I'm wanting this loop to execute an onload for each image in the array, and only proceed to the next iteration in the for loop once that's done. The onload is waiting for an image element to load before it executes.

 for (index = 0; index < lightboxImages.length; index++) { console.log('index :' + index); // when the current image has finished loading lightboxImages[index].onload = function() { console.log('image ' + index + ' loaded.'); }; } 

There are only three elements in the array, and with my console.log statements I'm expecting something like this in the console:

index : 0 image 0 loaded. index : 1 image 1 loaded. index : 2 image 2 loaded. 

But instead I'm getting output like this:

index : 0 index : 1 index : 2 image 3 loaded. image 3 loaded. image 3 loaded. 

How can I prevent iteration until the onload is finished?

I also tried something like this:

// just an index variable as an example var counter = 2; toImage(i) { if (i == counter) { console.log('hooray'); } else { toImage(i++); } } toImage(0); 

This was causing a callstack issue that was crashing the page - I didn't include it in the original post, because I figured it was a whole separate issue, but maybe I should have in retrospect. I guess my attempt at recursion here is wrong.

15
  • 1
    See this question on why you are getting image 3 three times. Maybe that helps you to get a solution that doesn't need to stop iterating? Commented Mar 20, 2018 at 22:07
  • 1
    I did try a recursive function, but that was causing me problems, too - I'll add that to my code above Commented Mar 20, 2018 at 22:08
  • 1
    here's another, though dunno if the answer works stackoverflow.com/questions/6702735/preload-images-with-queuing Commented Mar 20, 2018 at 22:14
  • 1
    And here's a much older, more comprehensive/popular thread on the topic: stackoverflow.com/questions/901677/… Commented Mar 20, 2018 at 22:15
  • 1
    ++ -> ++i you forgot i, and want to increment it before it gets passed as a parameter. Commented Mar 20, 2018 at 22:20

4 Answers 4

1

An alternative is recursive calls:

function loop(lightboxImages, index) { if (index === lightboxImages.length) return; lightboxImages[index].onload = function() { console.log('image ' + index + ' loaded.'); loop(lightboxImages, ++index); }; } loop(lightboxImages, 0); 
Sign up to request clarification or add additional context in comments.

9 Comments

yeah I was doing index++ and not ++index - will give this a try and come back
@rpivovar ok ok :)
There is no need for recursion in this example.
@NickCoad can you explain?
See my answer. The issue @rpivovar has encountered is a common one caused by scope and variables being passed by reference. The usual solution is to use a closure to capture the variable.
|
1

You can solve this with a closure.

var someFakeImages = [ { onload: function () {} }, { onload: function () {} }, { onload: function () {} } ]; var onloadFunc = function(i) { return function () { console.log('image ' + i + ' loaded.'); } }; for (index = 0; index < someFakeImages.length; index++) { console.log('index: ' + index); // when the current image has finished loading someFakeImages[index].onload = onloadFunc(index); } someFakeImages[0].onload(); someFakeImages[1].onload(); someFakeImages[2].onload();

You can find out more about closures on the MDN website: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures

Basically with the above, you are creating a new function with a snapshot of the value of index at the time of assigning the onload handler. In your posted code, you are actually using the current value of index at the time of executing the onload handler.

Comments

0

Assigning to onload only assigns an event handler: it doesn't mean that the current thread stops and waits for the load to finish. I suggest using reduce and await instead.

function loadImages(lightboxImages) { lightboxImages.reduce(async (lastPromise, image, i) => { await lastPromise; return new Promise((resolve, reject) => { image.onload = () => { console.log(`image ${i} loaded.`); resolve(); }; }); }, Promise.resolve()); } 

1 Comment

Don't make it any more complicated than it needs to be with that reduce. If you are going to use async/await anyway, just loop directly (with for (const [i, image] of lightboxImages.entries())) and await each one.
0

You console.log is showing the CURRENT value of index. You see, the loop has finished and index is now at 3 when the onloads are fired. You need to pass the index as a LOCAL variable to the onload handler.

It is a common 'issue' for many programmers in dealing with asynchronous coding ;)

6 Comments

The explanation is fine, but your thought is wrong.
I am 52 years old and have owned/built IT/tech companies and employed programmers since 1986 (from Sri Lanka, India, USA, Canada, UK, Australia, etc), which 'thought' is wrong?
The code snippet that follows "I would think" doesn't work.
Yes, I know, It was just an example to get the person asking the question on the right track. That is why I said 'think'.... How about now :)
Still no. Maybe better explain your thought rather than posting a code snippet.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.