Use async functions for file-system

[FFKL]

Hello! Why do you use sync methods for fs? Maybe better change them to async?

[bahung1221]

Hi bro!

About sync methods of fs, in this case, i think change to async doesn't make sense because next lines still must wait to previous line was done, example in get function:

if (Fs.existsSync(cacheFile)) {
    data = Fs.readFileSync(cacheFile)
    data = JSON.parse(data)
  } else {
    return fn(null, null)
}

// Next lines

As you see, we must check the file is exist before we can read it, same for other lines, right?
Are you have any idea to improve it? Thank you so much!

Additional, in core files, i used callback, this is a mistake 😢but it worked fine.

[FFKL]

Async methods don't block execution thread. Sync methods don't allow process other async operations (like http request, timers) while fs works. Example with await/async syntax:

async () => {
  if (await Fs.exists(cacheFile)) {
    data = await Fs.readFile(cacheFile)
    data = JSON.parse(data)
  } else {
    // do something else
  }
}

It is possible to improve it by modern syntax and async) easy to read and easy to use.

[bahung1221]

Good morning bro

I understood, I previously thought that the expensive task like I/O, Network,... will be executed on a worker in worker pool, not the main node event loop, so it doesn't block the main thread (event loop). I read some article about that.

But by you notice, seems I have misunderstood the problem
I think i will create a benchmark for this issue.

Thank you so much

[bahung1221]

About callback, i think we will rewrite whole project to typescript as version 3, so don't care about callback syntax, lol 🤣

[bahung1221]

Ah, I understood, i totally wrong, the IO task still be execute on a worker, but the event loop still waiting to it result if it's synchronous.

You saved my life, lol. It should be release in v2.0.8, i will working on it tonight or maybe you may help me create a PR, I grateful for that.

Thank you so much

[bahung1221]

Hi bro @FFKL, i merged and add test hook for it, but we still have a problem in fileStore constructor, we can't await constructor, so if we execute a cache method immediately after init it. Example:

fileCache.init()

await fileCache.set('key', { foo: 'bar' }) // FileStore cache instance still initializing

In this case, i think we should use sync method in constructor, it will not take too much cost because we just init it once. So should i change constructor to use sync method (same with old constructor)?

Thank you so much

[FFKL]

You're right. I missed It. But sync methods in constructor it's not good decision too. 😄 I think, for initialization we should use init() method for each type of caches. Now redis cache also use async functions for auth.

[bahung1221]

I missed redis constructor, too, lol.

It's ok, we can implement separate init method for each type of caches, and it should be async.
I think i will implement it in the next day, i bother you too much 🤣

Thank bro

[bahung1221]

Hi bro @FFKL ,
Version 2.0.8 was published to npm, I separated init method for each cache type.

Thank you for your supported 😄

[FFKL]

@bahung1221 cool! Good job 👍