0

I am trying to iterate over the selected files in input, i got a weird bug that happens when i switch the below statement

Object.keys(event.target.files).forEach(key => {
   const currentFile = event.target.files[key]
}

to

for (let key in event.target.files) {
   const currentFile = event.target.files[key]
}  

I don't understand whats the difference between these two statements, the first one works but the second one doesn't (results in weird bug where an object inside the loop becomes null.

Here is the full code :

onChange= (event) => {
        const selectedFilesObject = event.target.files
        let selectedFilesCount = Object.keys(selectedFilesObject).length

        if (selectedFilesCount > 0) {
            this.setState({ loading: true })
            this.props.onLockdownChange(true)

            let readFilesResult = {}

            for (let key in selectedFilesObject) {
                const currentFile = selectedFilesObject[key]

                readFileAsByteArray(currentFile)
                    .then(response => {
                        readFilesResult[key] = {
                            bytes: response,
                            name: currentFile.name,
                            type: currentFile.type,
                        }
                    })
                    .catch(() => {
                        readFilesResult[key] = null                        
                    })
                    .finally(() => {
                        selectedFilesCount = selectedFilesCount - 1
                        if (selectedFilesCount === 0) {
                            this.onReadingFilesFinished(readFilesResult)
                        }
                    })
            }
        }
    }  


export const readFileAsByteArray = (file) => {
    return new Promise((resolve, reject) => {
        var reader = new FileReader();
        var fileByteArray = [];
        reader.readAsArrayBuffer(file);
        reader.onloadend = (evt) => {
            if (evt.target.readyState == FileReader.DONE) {
                var arrayBuffer = evt.target.result,
                    array = new Uint8Array(arrayBuffer);
                for (var i = 0; i < array.length; i++) {
                    fileByteArray.push(array[i]);
                }

                resolve(fileByteArray)
            }
        }

        reader.onerror = error => {
            reject()
        };
    })
}  

I just need to understand why using for loop causes readFilesResult to have a null length! but using object.keys().forEach doesn't!

Ali Ahmadi
  • 701
  • 6
  • 27
  • https://stackoverflow.com/questions/10929586/what-is-the-difference-between-for-and-foreach#:~:text=The%20biggest%20differences%20are%20that,error%20in%20a%20foreach%20loop. – Kasey Chang Jun 10 '20 at 19:32
  • what is `event.target.files` ? – Ethan Lipkind Jun 10 '20 at 19:37
  • @KaseyChang I get the "difference" i just don't see what makes the second statement don't work for me. – Ali Ahmadi Jun 10 '20 at 19:38
  • @EthanLipkind thats the event of html `input ` element when it changes, it will contain the filename of the selected files, i just added the full code. – Ali Ahmadi Jun 10 '20 at 19:43
  • is the event mutable? Can the value for it CHANGE during the execution of the loop? – Kasey Chang Jun 10 '20 at 19:48
  • "*results in weird bug where an object inside the loop becomes null*" - could you be a bit more specific, please? – Bergi Jun 10 '20 at 19:49
  • Btw, you really should use `Promise.all` instead of that `selectedFilesCount` thing. – Bergi Jun 10 '20 at 19:49
  • @Bergi `readFilesResult` becomes `{length: null, item: null}`, by the way Pirhan solved the issue, meaning if i check the key using `hasOwnProperty`, the bug doesn't occur, of course i still don't fully understand why. – Ali Ahmadi Jun 10 '20 at 19:50
  • @KaseyChang There is no mutation in the code – Bergi Jun 10 '20 at 19:50
  • @Bergi `Promise.all`, thanks for pointing that out, i will use `Promise.all` in the future. – Ali Ahmadi Jun 10 '20 at 19:51

2 Answers2

3

You're enumerating a FileList here, which as a DOM collection has item and length properties. You should use neither Object.keys nor a for … in loop here, rather a for (let i=0; i<files.length; i++) iteration like on any array-like object. See also Why is using for…in on arrays such a bad idea?.

Or just convert it into an array right away:

onChange = (event) => {
    const selectedFiles = Array.from(event.target.files)
//                        ^^^^^^^^^^
    if (selectedFiles.length > 0) {
        this.setState({ loading: true })
        this.props.onLockdownChange(true)

        Promise.all(selectedFiles.map(currentFile =>
            readFileAsByteArray(currentFile).then(response => ({
                bytes: response,
                name: currentFile.name,
                type: currentFile.type,
            }), err => null)
        )).then(readFilesResult => {
            this.onReadingFilesFinished(readFilesResult)
        })
    }
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • 1
    Now i fully understand what happend of course i think its kinda weird using object which has a length! they should have used an array in the first place :D, but now i understand why the unexpected result happend, thanks, by the way nice job refactoring the code, its much more beautiful now – Ali Ahmadi Jun 10 '20 at 20:00
2

The difference between these two loops comes down to whether an object actually has a property, or inherited a property.

For example, if you have an object that inherited a property; Object.keys will not return it, but a for in loop will. Which is why you need to call hasOwnProperty method for each key you have in a for in loop.

In a more detailed way; for in loop is not bothered by the prototype chain.

// lets create a class called "Example".

class Example{
   constructor(a,b) {
      this.a = a;
      this.b = b;
   }
}

let instance = new Example(1,2);
for(let key in instance) {
    console.log(key); // will only print "a" and "b". Which is to be expected.
}

// Let's add a member to all of the objects that are instantiated from the class Example using Example's prototype.

Example.prototype.newMember = "I exist";

// Let's run the for in loop again

for(let key in instance) {
    console.log(key); // Prints "a", "b", and "newMember". The instances that are coming from "Example" doesn't have this... What happened there?
}

Turns out when you run a for-in loop, the loop doesn't recognize the prototype properties, and runs on all of them. Which is why I suggested running hasOwnProperty check on your example. hasOwnProperty checks to see if the member actually has a property, or is it a prototype property.

Does that make any sense?

Pirhan
  • 186
  • 1
  • 12
  • This is it, if i add `hasOwnProperty` my code gets fixed!, but i still don't understand what is happening, can u edit ur example and use something other than `window` ?, maybe build an object ur self so i can see what is happening. – Ali Ahmadi Jun 10 '20 at 19:48
  • Ask me if you have any other questions. – Pirhan Jun 10 '20 at 20:15