statement after for loop runs before for loop is done running

Question

why is objlist being resolved before loop has completed. Since the for loop is sync, I expected resolve to be run after my for loop has completed, but list with empty object is being resolved.

 function myFunction(path,files){
        return new Promise(function(resolve,reject){
            let objlist=[];
            files.forEach(function(file,index){
                console.log(file)
                objlist[index]={};
                fs.stat(path + '\\' + file, function(err,stats){
                    if(err){
                        console.log(err.code)
                        console.log(err)
                        throw err;
                    }
                    if(stats.isDirectory()){
                        objlist[index].file = 'folder'
                    }
                    else if(stats.isFile()){
                        objlist[index].file = 'file'
                    }
                    objlist[index].name = file
                    console.log(objlist[index].name) //gives correct output
                    objlist[index].size = stats.size
                });
            })
            console.log(objlist); //gives list of empty objects
            resolve(objlist);

        });
    }

Can't you just use the form `for ( var i = 0; i < files.length; i++ )` ? — Jeff Breadner, Mar 03 '18 at 18:54
@JeffBreadner I used that too but that didn't resolve the problem. — Abhishek Anand, Mar 03 '18 at 19:01

gwintrob · Answer 1 · 2018-03-03T23:09:51.203

0

forEach expects a function and executes it synchronously (see JavaScript, Node.js: is Array.forEach asynchronous?). fs.stat is async so there's no guarantee that all iterations will finish before calling resolve(objlist).

Here's a suggestion:

const objlist = await Promise.all(files.map(async file => {
  // return a promise
}))
resolve(objlist)

I like this article on promises and ES6 for a primer: https://developers.google.com/web/fundamentals/primers/promises

Edited thanks to feedback from https://stackoverflow.com/users/6351322/guijob

edited Mar 03 '18 at 23:09

answered Mar 03 '18 at 18:52

gwintrob

3,253
2
18
14

Whoops, big time brain fart. Thanks @guijob. Does my edit make sense? – gwintrob Mar 03 '18 at 19:00
In fact, forEach just expects a function and will execute it synchronously for each element of array, but if this function has async operations then you have to handle their responses manually. – guijob Mar 03 '18 at 19:07
it says unexpected identifier Promise.all – Abhishek Anand Mar 03 '18 at 19:20
You need to be using ES6 and have this inside an `async` function. – gwintrob Mar 03 '18 at 23:08

Jeff Breadner · Answer 2 · 2018-03-03T19:28:36.497

The fs.stat call is not synchronous because it uses a callback. Your forEach loop will launch all of the fs.stat processes and then continue, but it will not (nor can you make it) wait for all of the callbacks to be called before it carries on.

I think an approach like the one below will work. fs-extra is a good promisified version of fs, and bluebird is a promise library that extends standard Promises. I'm not sure if that is necessary, you didn't say which version of Node you're using; I'm using it to get access to the each method. Try it without Bluebird, it might work.

var fs = require('fs-extra');
var Promise = require('bluebird');
global.Promise = Promise;


function myFunction(path,files){
  return Promise
    .each(files, function(file) {
      return fs
        .stat(path + '\\' + file)
        .then(function(stats) {
          var result = {
            name: file,
            size: stats.size
          };
          if ( stats.isDirectory() ) result.file = 'folder';
          if ( stats.isFile() ) result.file = 'file'
          return Promise.resolve(result);
        });
    })
    .tap(console.log)
    .catch(function(err) {
      console.log(err.code);
      console.log(err);
      throw err;
    });
}

score 0 · Accepted Answer · answered Mar 03 '18 at 19:20

If you don't want to use await you can either use Promise.all or a counter to know when all fs.stat has resolved:

Using only Promise.all:

 function myFunction(path,files){
    return Promise.all(files.map(function(file,index){
            console.log(file)
            var obj = {};
            fs.stat(path + '\\' + file, function(err,stats){
                if(err){
                    console.log(err.code)
                    console.log(err)
                    throw err;
                }
                if(stats.isDirectory()){
                    obj.file = 'folder'
                }
                else if(stats.isFile()){
                    obj.file = 'file'
                }
                obj.name = file
                console.log(objlist[index].name) //gives correct output
                obj.size = stats.size
                return obj;
            });
        }))
    .then(objList => {
        console.log(objlist); //gives list of empty objects
        resolve(objlist);
    });
}

Or using a counter:

 function myFunction(path,files){
    return new Promise(function(resolve,reject){
        let objlist=[];
        var counter = files.length;
        files.forEach(function(file,index){
            console.log(file)
            objlist[index]={};
            fs.stat(path + '\\' + file, function(err,stats){
                counter--;
                if(err){
                    console.log(err.code)
                    console.log(err)
                    throw err;
                }
                if(stats.isDirectory()){
                    objlist[index].file = 'folder'
                }
                else if(stats.isFile()){
                    objlist[index].file = 'file'
                }
                objlist[index].name = file
                console.log(objlist[index].name) //gives correct output
                objlist[index].size = stats.size
                if(counter == 0) {
                    console.log(objlist); //gives list of empty objects
                    resolve(objlist);
                }
            });
        })
    });
}

statement after for loop runs before for loop is done running

3 Answers3