yuanchuan / node-watch

A wrapper and enhancements for fs.watch
https://npm.im/node-watch
MIT License
340 stars 44 forks source link

Calling close() before the watcher is ready does nothing #105

Closed filmaj closed 3 years ago

filmaj commented 3 years ago

I am testing another module that relies on node-watch. Some pseudo-code for how this module works looks like:

module.exports = function(callback) {
  let watcher = watch(process.cwd(), { recursive: true })
  watcher.on('change', /* some stuff here */)
  watcher.on('error', /* other stuff here */)
  if (callback) callback(null, { watcher })
}

It's a bit more complex but the above encapsulates the gist of it. My (tape-based) test looks something like this (pseudo-code again):

test('test', t => {
  myModule(function(err, { watcher }) {
    t.pass('module initialized')
    watcher.close()
    t.end()
  })
})

If I run this, my test script does not terminate.

After digging around, putting console.logs all over the place in node-watch and exposing the full internal state of the watcher in the expose() method, I realized that after my test called close(), the watcher's _isReady was false but _isClosed was true.

I can work around this by either waiting for the ready event on the watcher within either the test or my module before calling back, but I do feel like it was worth filing this issue in case you think it is worth catching this edge case.

In my opinion, it would be nice if calling close() soon after defining a watcher (and within the same tick) would work as expected and close / clean up appropriately. Looking through the code, it looks like the current behaviour is happening because some setup and teardown activity is postponed until the next tick, so I realize this may be easier said than done 😄

yuanchuan commented 3 years ago

Thanks for reporting this issue.

It's happening because there's an async detection for recursive availability.

The PR fix this issue by checking internal _isClosed before creating a watcher, which may prevent the unnecessary traversing on Linux.

yuanchuan commented 3 years ago

Please try 0.7.1