Issue in a for loop, get only the last item

Question

I´m using fileReader to read the content of all selected files. After reading them with the fileReader API, I append the content to the DOM. That works perfectly. It creates one p element per file.

Now I want to store each file content to local storage as well. Unfortunately, It stores only the last item. What´s going wrong? Thank you for your tips.

JS

$("input[name='uploadFile[]']").on("change", function() {

    var files = !!this.files ? this.files : [];
    if (!files.length || !window.FileReader)
        return;

    for (var i = 0; i < files.length; i++) {

        (function(file) {
            var name = file.name;
            var reader = new FileReader();
            reader.onload = function(e) {
                var textObject = event.target.result.replace(/\r/g, "\n");
                var textHTML = event.target.result.replace(/\r/g, "<br/>");

                var text = e.target.result;
                var p = document.createElement("p");
                p.innerHTML = textHTML;
                $('#results').append(p);    
                localStorage.setItem('letter'+ i, JSON.stringify(textObject));
            };

            reader.readAsText(file, 'ISO-8859-1');
        })(files[i]);

    }

});

score 2 · Accepted Answer · answered Nov 25 '16 at 22:37

The problem is that, by the time the onload is fired, i has been changed by the loop. This means that localStorage.setItem('letter'+ i will always refer to the last element in the array. You actually have the correct fix already in place -- the immediately invoked function expression -- but you need to add i as a parameter as well.

    (function(file, index) {
        var name = file.name;
        var reader = new FileReader();
        reader.onload = function(e) {
            var textObject = event.target.result.replace(/\r/g, "\n");
            var textHTML = event.target.result.replace(/\r/g, "<br/>");

            var text = e.target.result;
            var p = document.createElement("p");
            p.innerHTML = textHTML;
            $('#results').append(p);    
            localStorage.setItem('letter'+ index, JSON.stringify(textObject));
        };

        reader.readAsText(file, 'ISO-8859-1');
    })(files[i], i);

That rocks! Thank you so much lonesomeday – mm1975 Nov 25 '16 at 22:45 — mm1975, Nov 25 '16 at 22:45

score 0 · Answer 2 · answered Nov 26 '16 at 11:03

There is also another solution if you can use let

let allows you to declare variables that are limited in scope to the block, statement, or expression on which it is used. This is unlike the var keyword, which defines a variable globally, or locally to an entire function regardless of block scope.

Here is what it could look like:

for (let i = 0; i < files.length; i++) {
    let file = files[i];
    let name = file.name;
    let reader = new FileReader();

    reader.onload = function(e) {
        var textObject = e.target.result.replace(/\r/g, "\n");
        var textHTML = e.target.result.replace(/\r/g, "<br/>");

        var text = e.target.result;
        var p = document.createElement("p");
        p.innerHTML = textHTML;
        $('#results').append(p);    
        localStorage.setItem('letter'+ i, JSON.stringify(textObject));
    };

    reader.readAsText(file, 'ISO-8859-1');
}

Issue in a for loop, get only the last item

2 Answers2