49

I'm using FileReader API to read files on local.

<input type="file" id="filesx" name="filesx[]" onchange="readmultifiles(this.files)" multiple="" />

<script>
function readmultifiles(files) {
    var ret = "";
    var ul = document.querySelector("#bag>ul");
    while (ul.hasChildNodes()) {
        ul.removeChild(ul.firstChild);
    }
    for (var i = 0; i < files.length; i++)  //for multiple files
    {
        var f = files[i];
        var name = files[i].name;
        alert(name);
        var reader = new FileReader();  
        reader.onload = function(e) {  
            // get file content  
            var text = e.target.result;
            var li = document.createElement("li");
            li.innerHTML = name + "____" + text;
            ul.appendChild(li);
        }
        reader.readAsText(f,"UTF-8");
    }
}
</script>

If input includes 2 files:

file1 ---- "content1"
file2 ---- "content2"

I get this output:

file2__content1
file2__content2

How to fix code to display:

file1__content1
file2__content2
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
user384241
  • 549
  • 1
  • 9
  • 12

6 Answers6

103

The problem is you're running the loop now but the callbacks you are setting are getting run later (when the events fire). By the time they run, the loop is over and remains at whatever the last value was. So it will always show "file2" in your case for the name.

The solution is to put the file name inside a closure with the rest. One way to do this is create an immediately-invoked function expression (IIFE) and pass the file in as a parameter to that function:

for (var i = 0; i < files.length; i++) { //for multiple files          
    (function(file) {
        var name = file.name;
        var reader = new FileReader();  
        reader.onload = function(e) {  
            // get file content  
            var text = e.target.result; 
            var li = document.createElement("li");
            li.innerHTML = name + "____" + text;
            ul.appendChild(li);
        }
        reader.readAsText(file, "UTF-8");
    })(files[i]);
}

Alternately, you can define a named function and call it as normal:

function setupReader(file) {
    var name = file.name;
    var reader = new FileReader();  
    reader.onload = function(e) {  
        // get file content  
        var text = e.target.result; 
        var li = document.createElement("li");
        li.innerHTML = name + "____" + text;
        ul.appendChild(li);
    }
    reader.readAsText(file, "UTF-8");
}

for (var i = 0; i < files.length; i++) {
    setupReader(files[i]);
}
Ben Lee
  • 52,489
  • 13
  • 125
  • 145
  • I'm longing for further details on the explanation. – srph Jan 12 '15 at 18:05
  • 2
    @srph It has to do with javascript's variable scope. It creates closure for nested function and since the value of i changes each iteration, by the time the nested function is called it will get the last value of the array. So how Ben Lee modified the code was to make sure the function reads the file during the loop so it maintains the indices. I hope this made sense. (This is nicely described in [Effective Javascript](http://www.amazon.com/Effective-JavaScript-Specific-Software-Development/dp/0321812182) ) – KoalaD May 17 '15 at 20:22
6

Edit: Just use let instead of var in the loop. That fixes the issue OP had (but was only introduced in 2015).


Old answer (An interesting workaround):

While it is not exactly robust or future-proof, it is worth mentioning that this can also be achieved by adding a property to the FileReader object:

var reader = new FileReader();
reader._NAME = files[i].name; // create _NAME property that contains filename.

Then access it through e within the onload callback function:

li.innerHTML = e.target._NAME + "____" + text;


Why this works:

Even though the reader variable is replaced multiple times during the loop like i, the new FileReader object is unique and remains in memory. It is accessible within the reader.onload function through the e argument. By storing additional data in the reader object, it is kept in memory and accessible through reader.onload via e.target event argument.

This explains why why your output is:

file2__content1
file2__content2

and not:

file1__content1
file2__content2

The content is displayed correctly because e.target.result is a property within the FileReader object itself. Had FileReader contained a filename property by default, it could have been used and this whole mess avoided entirely.


A word of caution

This is called extending host objects (if I understand the difference between native objects...). FileReader is the host object that is being extended in this situation. Many professional developers believe doing this is bad practice and/or evil. Collisions may occur if _NAME ever becomes used in the future. This functionality isn't documented in any specification so it could even break in the future, and it may not work in older browsers.

Personally, I have not encountered any issues by adding additional properties to host objects. Assuming the property name is unique enough, browsers don't disable it, and future browsers don't change these objects too much, it should work fine.

Here are some articles that explain this quite well:

http://kendsnyder.com/extending-host-objects-evil-extending-native-objects-not-evil-but-risky/
http://perfectionkills.com/whats-wrong-with-extending-the-dom/

And some article on the problem itself:

http://tobyho.com/2011/11/02/callbacks-in-loops/

bryc
  • 12,710
  • 6
  • 41
  • 61
6

Instead of using var, use let as the declared variable only be used in one loop.

for (let i = 0; i < files.length; i++)  //for multiple files
    {
        let f = files[i];
        let name = files[i].name;
        alert(name);
        let reader = new FileReader();  
        reader.onload = function(e) {  
            // get file content  
            let text = e.target.result;
            let li = document.createElement("li");
            li.innerHTML = name + "____" + text;
            ul.appendChild(li);
        }
        reader.readAsText(f,"UTF-8");
    }
Kyo Kurosagi
  • 2,177
  • 1
  • 18
  • 18
4

You can make a promise/callback for reading the file in the loop.

Promise-

  fileBase64(file) {
    return new Promise((resolve, reject) => {
      let reader = new FileReader();
      reader.readAsDataURL(file);
      reader.onload = function() {
        resolve(reader.result);
      };
      reader.onerror = function(error) {
        reject(error);
      };
    });
  }

I am calling this function on onClick

  onClick = async () => {
    for (var i = 0; i < this.state.company_bank_statement_array.length; i++) {
      let file = document.getElementById(
        this.state.company_bank_statement_array[i]
      );
      let fileData = await this.fileBase64(file.files[0]);
      this.state.bankStatements.push({
        data: fileData,
        filename: file.files[0].name,
      });
    }
  };
Ankit Kumar Rajpoot
  • 5,188
  • 2
  • 38
  • 32
3

I had the same problem, solved it by using Array.from

let files = e.target.files || e.dataTransfer.files;

Array.from(files).forEach(file => {
 // do whatever
})
lee shin
  • 894
  • 11
  • 6
0

I think the best way to solve this problem is by recursively call a function that reads the blob file. So in my case I solve the problem with the following snippet code, maybe is a little complicated but it works in any scenario that I tried.

Notice that, I didn't pass the array and index as arguments. You need to call them with the object they belong to.

//Initialize blobs
var foo = new Blob(["Lorem ipsum dolor sit amet, consectetur adipiscing elit."], {
    type: 'text/plain'
});
var bar = new Blob(["Sed tristique ipsum vitae consequat aliquet"], {
    type: 'text/plain'
});
//Initialize array and index
var arrayOfBlobs = [foo, bar];
var arrayIndex = 0;

function fileRead () {
    var me = this;
    if (this.arrayIndex < this.arrayOfBlobs.length) {
        var reader = new FileReader();

        function bindedOnload(event) {
            console.log("bindedOnload called");
            console.log("reader results: ", event.target.result);
            this.arrayIndex++; //Incrument the index
            this.fileRead(); //Recursive call
        }
        //By Binding the onload event to the local scope we
        //can have access to all local vars and functions
        reader.onload = bindedOnload.bind(me);
        reader.readAsText(this.arrayOfBlobs[arrayIndex]);
    } else {
        //This will executed when finishing reading all files
        console.log("Finished");
    }
}

//Call the fileRead for the first time
fileRead();
yannisalexiou
  • 605
  • 12
  • 25