9
\$\begingroup\$

I wrote a small function that checks if an image or multiple images are loaded. For that purpose, I decided to use ES6 promises, but I'm not really sure if my way of handling errors is the best way.

const loadImg = function loadImg(src) {
    'use strict';

    const paths = Array.isArray(src) ? src : [src];
    const promise = [];

    paths.forEach((path) => {
        promise.push(new Promise((resolve, reject) => {
            const img = new Image();

            img.onload = () => {
                resolve({
                    path,
                    status: 'ok',
                });
            };

            // Call `resolve` even if the image fails to load. If we were to
            // call `reject`, the whole "system" would break
            img.onerror = () => {
                resolve({
                    path,
                    status: 'error',
                });
            };

            img.src = path;
        }));
    });

    return Promise.all(promise);
};

// Usage
loadImg([
    'IMG.png',
    'IMG.jpg',
    'IMG.jpg',
    'IMG.jpg',
]).then(function(e) {
    console.log(e);
});

It works, but I'm worried that using objects as error messages rather than reject has some disadvantages that I'm not aware of (e.g. potential memory leaks or people don't expect to see errors in then())? Is there any way I can improve this function?

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Why did you opt to not reject() on error? \$\endgroup\$ May 17, 2016 at 20:17
  • \$\begingroup\$ @MadaraUchiha If I did, the Promise would fail even if only one image in many, say 20, didn't load for some reason. Secondly, if two images out of 10 fail, there is (?) no way to display which of them failed. I can only show one path. So the error messages were kind of useless too. That's why I opted out, but I'm open to suggestions if there's a better way to solve this. \$\endgroup\$
    – mrksbnch
    May 17, 2016 at 20:31
  • \$\begingroup\$ That's fair. I can see the point. \$\endgroup\$ May 17, 2016 at 20:34

2 Answers 2

24
\$\begingroup\$

Well, you normally don't want to use the Promise constructor in your higher level code, you want to Promisify as low as possible. So let's create a function that checks for a single image, and resolves whenever you know the status of that image:

// When there's only one statement, you can drop the {} and the return
// x => y is equivalent to x => { return y; }
const checkImage = path =>
    new Promise(resolve => {
        const img = new Image();
        img.onload = () => resolve({path, status: 'ok'});
        img.onerror = () => resolve({path, status: 'error'});

        img.src = path;
    });

Now, with that Promise returning action, we can do some fancy things:

const loadImg = paths => Promise.all(paths.map(checkImage))

Or, if you want to get fancier

const loadImg = (...paths) => Promise.all(paths.map(checkImage));
// and call with
loadImg('path1', 'path2', 'path3');
// or
loadImg(...arrayOfPaths)

It's perfectly fine to not reject if there's no error. Yes, there was an error loading the image, but it isn't an exceptional situation, it's not something you'd throw because of. Your function is actually there to check if the image loaded fine or not.

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

Sounds like a use case for Promise.allSettled().

The Promise.allSettled() method returns a promise that resolves after all of the given promises have either fulfilled or rejected, with an array of objects that each describes the outcome of each promise.

Here is a jsfiddle with a modified version of your example, basically:

const loadImg = function loadImg(src) {
    'use strict';
    
    const paths = Array.isArray(src) ? src : [src];
    const promises = [];
    
    paths.forEach((path) => {
        promises.push(new Promise((resolve, reject) => {
            const img = new Image();
            
            img.onload = ()=>resolve(path);
            img.onerror = ()=>reject(path);     // reject() is not a problem.
            
            img.src = path;
        }));
    });
    
    return Promise.allSettled(promises);        // the important part.
};

// Usage
loadImg([
    'IMG.png',
    'IMG.jpg',
    'IMG.jpg',
    'IMG.jpg',
]).then((results) => results.forEach((result) => {
    console.log(result);
}));

Results in something like this:

{
  reason: "IMG.png",
  status: "rejected"
}
{
  status: "fulfilled",
  value: "IMG.jpg"
}
{
  status: "fulfilled",
  value: "IMG.jpg"
}
{
  status: "fulfilled",
  value: "IMG.jpg"
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.