Async FileReader help (please)



  • 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.



  • @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.



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



  • @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?



  • 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.



  • @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.



  • @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();


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



  • 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);
            })
        ; 
    
    }
    

Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.