1

How do I get the promises to execute only after concatenation is done?

var fs = require('fs');
var testFolder = './lib/';
var files = ['hello.txt', 'goodbye.txt'];
var contents = '';
//creating array of promises
let promises = files.map(async e => {
    return await fs.readFile(testFolder + e, "utf8", function (err, content) {
            contents += content + ".\n";
        });
 }
);
    console.log(promises);
    //this should happen last but "contents" is still empty string?
    Promise.all(promises).then(()=> console.log(contents));
Raphael
  • 11
  • 3

1 Answers1

1

fs.readFile doesn't return a promise.

You should use util.promisify to make it do so.

Furthermore, as mentioned by Patrick Hübl-Neschkudla in the comment below, there's no need to use async/await since readFile already returns a promise now and we're not processing the results until later.

Bergi also pointed out in the comments that fs.readFile callbacks aren't guaranteed to return in the order of creation. In fact that's the case for most fs operations because other processes can always interfere with yours by manipulating files at the same time - for example, what if somebody writes to a file you're about to read?.

You should perform the concatenation once all reads are finished instead of doing it while they are happening (as they might get out of sync).

This will also make sure all rejections are properly caught no matter which promise (file read) fails.

const readFile = util.promisify(fs.readFile);

Promise.all(
  files.map(e => readFile(testFolder + e, "utf8"))
)
.then(contentArray => contentArray.join('.\n'))
.then(console.log);
nem035
  • 34,790
  • 6
  • 87
  • 99
  • the async/await is not needed here – Patrick Hübl-Neschkudla Nov 15 '17 at 15:21
  • You should mention that this does not have a deterministic order of `content`s in the result string. – Bergi Nov 15 '17 at 15:28
  • @Bergi you're saying that I should mention that `fs.readFile` callbacks aren't guaranteed to return in the order of creation? – nem035 Nov 15 '17 at 15:33
  • @nem035 Yes, people might expect that from promises. One should simply do the concatenation after the `Promise.all` – Bergi Nov 15 '17 at 15:36
  • Actually, constructing the promises up-front and then sequentially awaiting them (using `await` or `then`) [is still an antipattern that can cause unhandled rejection warnings](https://stackoverflow.com/questions/46889290/waiting-for-more-than-one-concurrent-await-operation). You should either just use `Promise.all` (and concatenate afterwards) or read the files sequentially. – Bergi Nov 15 '17 at 15:52
  • 1
    @Bergi makes sense, i'll remove those suggestions. – nem035 Nov 15 '17 at 15:59