Is this the correct way to wrap readFileSync in a Promise

Question

The reason for the below code is to get rid of callback hell/pyramid of doom. I don't fully understand i/o blocking though yet.

'use strict';

var fs = require('fs');
var co = require('co');

co(function* () {

    var fileName = 'readme.txt';

    var str =
        yield new Promise(function (resolve, reject) {
            var result;
            try {
                result = fs.readFileSync(fileName, 'utf8');
            } catch (err) {
                reject(err);
            }
            resolve(result);
        });
    console.log('result readFileSync: ' + str);

});

All I'm expecting is a yes or no answer to be honest. Hope fully if no could someone give some details as I'm trying to learn properly about JavaScript sync/async and how to harness the power of Promises.

Why in the world would you try to wrap a promise around `rs.readFileSync()`? If you want async, use `fs.readFile()`. — jfriend00, Jan 24 '16 at 19:48
@jfriend00 Ever heard of `callback hell`/`pyramid of doom`? Go check it! — basickarl, Jan 24 '16 at 19:49
`fs.readFileSync` does not need callbacks at all. It's synchronous. There is no callback hell with `fs.readfileSync()`. I'm thinking that maybe you meant to ask about `fs.readFile()` which IS asynchronous and does involve callbacks. — jfriend00, Jan 24 '16 at 19:56

score 11 · Accepted Answer · answered Jan 24 '16 at 19:54

11

Short answer

No

Useful answer

If you want to wrap a file read operation, try to use the async versions of Node functions as much as possible. Using readFileSync with a promise gives you no advantage over using readFileSync on its own, because readFileSync blocks the process until it is done reading, which readFile does not.

A better solution would therefore be something like this:

'use strict';

var fs = require('fs');

var readFilePromise = function(file) {
  return new Promise(function(ok, notOk) {
    fs.readFile(file, function(err, data) {
        if (err) {
          notOk(err)
        } else {
          ok(data)
        }
    })
  })
}

readFilePromise('/etc/passwd').then(function(data) {
  // do something with the data...
})

answered Jan 24 '16 at 19:54

conradkleinespel

6,560
10
51
87

1 up also, One of the better answers I have received! – basickarl Jan 24 '16 at 20:15
1

Down voting this because it is not the correct way. – dman Jan 15 '18 at 01:00
1

@dman Promisify was added in Node 8.0.0 (source: https://nodejs.org/api/util.html#util_util_promisify_original), which was released 2017-05-30 (source: https://nodejs.org/en/download/releases/), I posted this answer in 2016. I think it's interesting to have your answer in mind for Node 8+. But for anything 8-, my answer seems like the way to go. – conradkleinespel Jan 28 '18 at 22:52

dman · Answer 2 · 2018-01-15T01:20:04.007

8

The correct way is to use the Node's native util library and promisfy fs.readFile:

const path = require('path');
const fs = require('fs');

const INDEX = path.join(__dirname, 'app', 'index.html');
const readFile = require('util').promisify(fs.readFile);

readFile(INDEX)
  .then(e => console.log(e.toString()))
  .catch(e => console.log('FOOBAR ' + e));

result:

one@dolphin:~/github/resume $ node toolchain/inject.js 
FOOBAR Error: ENOENT: no such file or directory, open '/home/one/github/resume/toolchain/app/index.html'

edited Jan 15 '18 at 01:20

answered Jan 15 '18 at 01:02

dman

10,406
18
102
201

2

Isn't promisfying with util just the same as wrapping it in a Promise? – basickarl Jan 15 '18 at 14:59
@KarlMorrison yes, but you asked the `correct` way. Methodology trumps all else. The same with array methods, you use them over loops. Outside the context of a promise and on a side note: We took ~13 lines and made them into one line. This is acceptable because `utils` is a native library and does not bloat the code base. It would be unacceptable to make 13 lines of code into 1 if it was done with a library that was not native, such as lodash, which would bloat the codebase. – dman Jan 15 '18 at 15:58
Don't get me wrong I agree with you, was just curious if anythnig else was in play here :) – basickarl Jan 15 '18 at 16:04
@KarlMorrison You can read more in the Node Util docs. Were you going to accept this as the answer or at the least upvote? – dman Jan 15 '18 at 18:34
You will certainly indeed get an upvote! :) conradkdotcom however did explain the essentials behind the sync vs. async approach process approaches, meaning that readFileSync and readFile should be used in different scenarios and cannot replace each other even when promisifyed. – basickarl Jan 16 '18 at 11:58

Is this the correct way to wrap readFileSync in a Promise

2 Answers2

Short answer

Useful answer

Linked