Async FileReader help (please)

Benjamin Hall

So I'm continuing my project, and want to allow inclusion of images. The part I'm struggling with is reading the files from disk and creating an object that maps the filename to the base64-encoded image data (to be sent to the server). I'd rather not upload the files directly, so straight up form submission is out.

Here's what I have (after cribbing mightily from SO):

function getImageData() {
    function getPromise(images, file){
        return new Promise(function(resolve, reject){
            var reader = new FileReader();
            reader.onload = function(evt){
                images[file.name] = evt.target.result;
            };
            reader.onerror = function(error) {
                reject(error);
            };
            reader.readAsDataURL(file);
        });
    }

    var uploadElement = $("#image-upload")[0].files,
        promises = [],
        images = {};
    for (var i=0; i < uploadElement.length; i++){
        promises.push(getPromise(images, uploadElement[i]));
    }
    promises.reduce(function(cur, next){
        return cur.then(next);
    }, Promise.resolve()).then(function(){
        return;
    }
    );
}

It's...not working. Specifically, the files seem to be loaded but aren't being attached to the object. I'm sure it's something having to do with the async nature of the filereader api, but I have no clue how to fix it. Thus--is there a better way of doing this? If there isn't, what the %^#^#^ am I doing wrong? Explanations of why it's wrong would also be appreciated so I can learn.

anotherusername

@Benjamin-Hall ...I don't see what it's supposed to be doing with them after loading them. Also the whole promises way of doing it seems unnecessarily complex.

The_Quiet_One

One thing I see is the resolve callback on the promise function is never called.

Benjamin Hall

@anotherusername Doh I had it half mutated from modifying a global object (a real WTF) to returning the images object. I need to attach that key-value object (key is the filename, value is the image data) to the json object I'm building so that I can upload the images + a bunch of other data all at once.

As for the promises--is there an easier way of making sure that when the function returns all the images have loaded and been inserted into the object?

anotherusername

It looks like you expect images to contain the images, but it isn't returned, and it's a local variable. It goes out of scope when getImageData() returns.

I'd do something like this... note that this logs the contents of each file to the console as a data URL, but it still doesn't do anything with it:

var images = {}, uploadElement = document.getElementById('image-upload');
for (var i = 0; i < uploadElement.files.length; ++ i) {
  var reader = new FileReader();
  reader.onload = function (event) {
    images[event.target.name] = event.target.result;
    // ...you need to actually DO something with images[] though...
    console.log(event.target.result); // verify that it's actually reading the file successfully...
  };
  reader.onerror = function (event) {
    // you can do whatever here...
  };
  reader.readAsDataURL(uploadElement.files[i]);
  reader.name = uploadElement.files[i].name;
}

edit: oh, is your next function supposed to do something with the results? ...yeah it still seems unnecessarily complicated.

Benjamin Hall

@The_Quiet_One Where should it be called? in the reduce function (instead of Promise.resolve()?) I'm a total noob as far as promises go.

The_Quiet_One

@Benjamin-Hall said in Async FileReader help (please):

@The_Quiet_One Where should it be called? in the reduce function (instead of Promise.resolve()?) I'm a total noob as far as promises go.

I would expect it to be called right here:

images[file.name] = evt.target.result;
resolve();

Benjamin Hall

@The_Quiet_One That seemed to have done it. It appears to be working now. Thanks!

Zecc

Here's my take on it.

// This is pretty much what you already had
function readFile(file){
    return new Promise(function(resolve, reject){
        var reader = new FileReader();
        reader.onload = function(evt){
            console.log("Just read", file.name);
            resolve(evt.target.result);
        };
        reader.onerror = function(err) {
            console.error("Failed to read", file.name, "due to", err);
            reject(err);
        };
        reader.readAsDataURL(file);
        // Would be sweet if readAsDataURL already returned a promise
    });
}

// You want this function to exist
function readMultipleFiles(files){
    var results = {};
    
    // You can shim Array.from for browsers that don't have it
    var promises = Array.from(files, function(file){
        return readFile(file)
            .then(function(data){
                results[file.name] = data;
                console.log(file.name, "added to results");
                // Implicitely return undefined. Data is already in results
            })
        ;
        // Automatic semicolon insertion means the semicolon above
        // is redundant for the interpreter. I put it for readability's sake.
    });
    
    // Yes, Promise.all is a thing. No need for Array#reduce tricks
    return Promise.all(promises)
        .then(function(_){ // Don't care about the array of undefineds
            console.log("Done");
            return results;
        })
    ;
}


// Main logic, cleanly separated
function doTheUploadYo(){

    var uploadElement = document.getElementById('image-upload');
    readMultipleFiles(uploadElement.files)
        .then(function(results){
            console.dir(results);
        })
        .catch(function(err){
            console.error("Well that sucks:", err);
        })
    ; 

}