Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pulsar-next] Nudge toward async native file watching #9

Open
savetheclocktower opened this issue Dec 27, 2024 · 0 comments
Open

[pulsar-next] Nudge toward async native file watching #9

savetheclocktower opened this issue Dec 27, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@savetheclocktower
Copy link

savetheclocktower commented Dec 27, 2024

pathwatcher still exists as a dependency because its consumers within Pulsar — including text-buffer — need a file-watcher that starts instantly:

const { File } = require('pathwatcher');
const fs = require('fs');
let fileToWatch = new File('foo.txt')
fileToWatch.onDidChange(() => {
  console.log('changed!');
});
fs.writeFileSync('foo.txt', 'stuff')
// expect "changed!" in the console

Notice how nothing goes async. This is hard. The file-watcher we’d like to use is nsfw, but it goes async both when first creating the file watcher and when adding a file to the watcher. The sample code above can't easily be converted to use nsfw.

But it gets worse: since all file subscriptions and unsubscriptions are assumed to happen synchronously, imagine how we handle a TextBuffer pointing to a new file on disk:

  • unsubscribe from the old path
  • immediately subscribe to the new path
  • immediately expect that a change at the new path will be detected

The fact that pathwatcher ever did this successfully borders on miraculous, but it’s probably because it never used the FSEvents API on macOS.

The rewrite and the workarounds

We had to rewrite pathwatcher for context-awareness so it could run in newer Electron. I had no idea how much of a “don’t touch it or you’ll break it” ball of code pathwatcher was. It had nan bindings, but also used libuv directly in most places. It was stubbornly context-unaware, and after trying to introduce it to a context-aware world for a couple of weeks, I decided we needed a different approach.

I was painfully aware of the synchronous-above-all API contract I needed to fulfill, and I can say that the rewrite is probably 95% of what it needs to be. The main area in which we have trouble is the scenario above: immediate unsubscription and resubscription.

I’ve done a lot of experimentation, but I haven’t yet been able to get it to the point where we can unsubscribe on one line of code and re-subscribe on the next — at least in the case where it’s the only listener. That’s because, when there are no listeners left (even for a brief moment), we stop the watcher altogether, and starting that machinery up again in a prompt fashion appears to be a challenge.

Luckily, this scenario is pretty unusual in real life; unluckily, it comes up predictably often in unit tests. We can usually work around it by going async in an afterEach hook — an await wait(0) is typically enough. (We can avoid the unsubscribe/resubscribe in the scenario above by adding the new file watcher before we stop the old one, but workarounds aren't always an option.)

The ultimate goal

We don’t want to be in the business of maintaining a file-watching API. We want to use nsfw. But anything in the codebase that currently isn’t using nsfw isn’t using it because it expects a synchronous API.

text-buffer needs to be nudged in the right direction here. It will be a bit painful, but we need to tweak the API (while preserving backward-compatibility as much as possible) to add asynchronousness to more methods.

There will probably be a need for the legacy synchronous API indefinitely just because it’s part of the API we expose to community packages, but at the very least our core codebase should expect to wait for file-watchers to attach — and text-buffer is the ideal first candidate.

@savetheclocktower savetheclocktower added the enhancement New feature or request label Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant