Ensure watchPath "de-normalizes" filesystem event paths if opted into…

[savetheclocktower]

…so that users of watchPath don't have to do their own realpath resolution.

I've been going through the PulsarNext package tests in order to fix some regressions. Many of them are related to file-change monitoring in one way or another. For all its faults, the old pathwatcher was able to start watching a file for changes in a totally synchronous manner (don't ask me how), which made it quite easy to write tests around. The new pathwatcher has the same synchronous API, but cannot guarantee instant subscription in an environment (like a spec suite!) where unsubscription and resubscription happen in rapid succession. This is annoying.

For this reason and others — like not wanting to be maintainers of our own file-watching library — we have a medium-term goal to replace our own pathwatcher library with nsfw across the board. In fact, the goal is convert usages of pathwatcher to a newer file-watching interface defined by Atom called watchPath; the idea is that we'd be further away from the bare metal of a particular file-watching library and could more easily migrate to a different one in the future should the need arise.

For that reason, I’ve tried to seize opportunities to convert usages of pathwatcher to watchPath.

But there’s a problem I’ve had to solve each time. Consider this pathwatcher code:

const userSnippetsFile = new File(userSnippetsPath)
userSnippetsFileDisposable.add(userSnippetsFile.onDidChange(
  () => this.handleUserSnippetsDidChange()
))
userSnippetsFileDisposable.add(userSnippetsFile.onDidDelete(
  () => this.handleUserSnippetsDidChange()
))
userSnippetsFileDisposable.add(userSnippetsFile.onDidRename(
  () => this.handleUserSnippetsDidChange()
))

Here’s the rough analog of that code using watchPath:

watchPath(path.dirname(userSnippetsFile), {}, (events) => {
  for (let event of events) {
    if (event.path === userSnippetsFile || event.oldPath === userSnippetsFile) {
      this.handleUserSnippetsDidChange();
      break;
    }
  }
})

This reflects the fact that watchPath watches directories, not individual files.

But this doesn’t work out of the box on my machine — the specs that test this behavior will fail.

Here’s why: the specs rely on the temp module to generate directories within /tmp to use as temporary ATOM_HOMEs and the like… but on macOS, everything in /tmp is actually symlinked to /private/tmp! So watchPath will normalize the desired path to something that starts with /private/tmp, and all the filesystem events within the callback will have path and oldPath properties that start with /private/tmp. So all the equality checks will fail.

But this isn’t just a problem with specs, since symlinks can happen anywhere. So it needs fixing in the general case.

There’s one way to fix this that works just fine as long as you already know that userSnippetsFile exists and is an absolute path:

let realUserSnippetsFile = fs.realPathSync(userSnippetsFile)
watchPath(path.dirname(realUserSnippetsFile), {}, (events) => {
  for (let event of events) {
    if (event.path === realUserSnippetsFile || event.oldPath === realUserSnippetsFile) {
      this.handleUserSnippetsDidChange();
      break;
    }
  }
})

But it’s not nice to make each consumer do this work when watchPath itself could handle it instead. That’s what this PR does:

• We add a new option to watchPath (which previously had no options — just an empty placeholder for future options!) called realPaths. It defaults to true.
• This new functionality is enabled when someone opts into it via { realPaths: false }. When they do…
  • Everything about watchers still works identically. Each call to watchPath creates a PathWatcher instance, and multiple PathWatchers are likely to share a single instance of NativeWatcher.
  • One filesystem event as generated by NativeWatcher will contain the canonical path on disk for the file and can be given to multiple PathWatchers…
  • …but each PathWatcher knows the path the user specified and can generate its own custom “denormalized” version of the event with the paths the user might expect. (If no symlinks are involved, then PathWatcher will re-use the existing event.)

This is how watchPath should always have worked. I tried very hard to convince myself to make this the new default behavior… but it would theoretically be a breaking change, even though I doubt it would affect many packages.

At any rate, lots of packages won’t be using this API directly; they’ll use the File and Directory classes that are exported from atom. Those have been around for much longer and have built-in change monitoring via onDidChange, onDidDelete, et cetera.

But atom gets File and Directory straight from pathwatcher. If we really want to reduce reliance on pathwatcher, we’ll need to build new File and Directory wrappers that can behave identically enough but use watchPath. I’ll see about doing that as well.