fix: prefer async fs operations for less event-loop blocking

[michael-go]

This will be especially useful when used in a web-server, rather than CLI, to not block other requests in-case of slow/network storage.

[michael-go]

Oh noes ... util.promisify is node 8 only - can easily write my own "promise" wrapper for these 2 functions ... should I?

[miiila]

@michael-go I hope you don't mind me stepping in - please let me know, WDYT about this approach.

[darscan]

So... I personally don't feel that this gives us much. The actual operations are still synchronous under the hood. Allowing for a single event loop's worth of breathing room is not going to actually have any impact.

@miiila I think, as an exercise, the hand-rolled promisify is a good one, but in the end it's just more code.

@michael-go If Node 8 promisify was actually available to us, then I'd "maybe" be OK with it, but even still I'd question the benefit.

Just my opinions, feel free to disregard :)

[miiila]

Oh my, of course! I shouldn’t do this late in the evening :-) On 14 Aug 2018, 23:37 +0100, Shaun Smith notifications@github.com, wrote:

@darscan commented on this pull request. In lib/index.ts:

return await buildDepTree(targetFile, lockFile); } + +function promisify(func): any {

• return (...args: any[]) => {
• return new Promise((resolve, reject) => {
• return func(...args, (err, data) => {
• if (err) {
• reject(err);
• }
• resolve(data); p.s. it's not good to both reject and resolve a promise. I'd return from the rejection in the error case — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[lili2311]

I will be closing this one, let's re-open if we decide we need this in

[michael-go]

@darscan - it's not only a single (double actually, as there are 2 files) event loop breathing room - during the IO nothing else happens if it's Sync. Quoting from an article @adrukh recently shared (https://nodesource.com/blog/node-js-performance-monitoring-part-3-debugging-the-event-loop/):

Sync() methods that you find in the Node.js core API, particularly in the ‘fs’ module (e.g. fs.readFileSync()). These methods have the unfortunate property of running *inside your event loop, on the same thread as your JavaScript. Choosing this method means that with each cycle of the event loop there’s a pause until execution completes; nothing else can process in the meantime. You should generally avoid all synchronous core methods in your application for this reason.

cc @miiila @lili2311