Looping through files for FileReader, output always contains last value from loop

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

Solution 1:

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

Solution 2:

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/

Solution 3:

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