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

Support @parcel/watcher #13593

Open
4 tasks done
rtsao opened this issue Jun 21, 2023 · 46 comments
Open
4 tasks done

Support @parcel/watcher #13593

rtsao opened this issue Jun 21, 2023 · 46 comments
Labels
enhancement New feature or request

Comments

@rtsao
Copy link
Contributor

rtsao commented Jun 21, 2023

Description

Chokidar is slow and known to have issues with file descriptor limits when working with large projects. Parcel's watcher is much faster out of the box and additionally supports watchman as a backend, which improves support for Windows and minimizes overhead for projects already using watchman.

Nuxt recently added support for @parcel/watcher: nuxt/nuxt#20179

Suggested solution

Add option to use @parcel/watcher

Alternative

No response

Additional context

Related issues:

Twitter discussion https://twitter.com/patak_dev/status/1649060364431028226

Validations

@bluwy
Copy link
Member

bluwy commented Jun 22, 2023

We've discussed this before (Discord thread) and are open to trying it in Vite 5. The main blocker for me is that @parcel/watcher isn't publishing precompiled binaries like lightningcss and esbuild, so it brings a larger published size by default.

@bluwy bluwy added enhancement New feature or request and removed enhancement: pending triage labels Jun 22, 2023
@bluwy bluwy added this to the 5.0 milestone Jun 22, 2023
@silverwind
Copy link

We've discussed this before (Discord thread) and are open to trying it in Vite 5. The main blocker for me is that @parcel/watcher isn't publishing precompiled binaries like lightningcss and esbuild, so it brings a larger published size by default.

Vite should not depend on a C++ compiler being present as that is a major pain point for users.

@bluwy
Copy link
Member

bluwy commented Jun 22, 2023

How so? Vite is already using esbuild. And has ecosystem support for SWC and lightningcss.

@sapphi-red
Copy link
Member

@parcel/watcher isn't publishing precompiled binaries like lightningcss and esbuild

@silverwind I think you misunderstood this comment. I guess you understood that comment as "@parcel/watcher isn't publishing precompiled binaries" and "@parcel/watcher uses node-gyp / C++ compiler".
But I believe @bluwy meant to say "@parcel/watcher isn't publishing precompiled binaries for each OS/arch in a separate package like lightningcss and esbuild does".

@silverwind
Copy link

Ah, so it is one big package of precompiled binaries with all os and archs in the package. Not ideal for install performance, but at least requires no install-time compilation, so it may be acceptable.

@gajus
Copy link
Contributor

gajus commented Jun 22, 2023

Just for my curiosity, any reason for not considering Turbowatch?

Mentioned in the original thread #12495 (comment)

It uses fs.watch under the hood by default and otherwise fallsback to other mechanisms, including Watchman.

@silverwind
Copy link

Just for my curiosity, any reason for not considering Turbowatch?

Mentioned in the original thread #12495 (comment)

It uses fs.watch under the hood by default and otherwise fallsback to other mechanisms, including Watchman.

From a maintenance standpoint, a pure JS module would be much better than C++ modules, which tend to require constant maintenance to not break because of ABI changes etc. It you check the issue tracker of parcel-bundler, there are a number of open issues about non-working platforms.

@gajus
Copy link
Contributor

gajus commented Jun 22, 2023

That + it is very easy to add custom backends. Here is an example chokidar backend.

The default backend basically checks if fs.watch can be used, and if it can, then it uses it, or otherwise fallsback to chokidar.

The other thing worth mentioning is that Turbowatch implements file hashing on top of file change triggers provided by the backend. In practice, we've found that this dramatically reduces the number of unnecessary re-renders as compared to mtime based systems.

@devongovett
Copy link

devongovett commented Jun 23, 2023

Releasing separate npm packages per platform has been on my list for @parcel/watcher for a while, just haven't had time yet. If someone wants to contribute that, it would be helpful, otherwise I will get to it at some point soon hopefully.

which tend to require constant maintenance to not break because of ABI changes etc

This particular change is very rare actually. Node-API is ABI stable, so should not break between versions of node. There are a few very obscure platforms that we haven't implemented backends for yet, or we need to provide pre-builds for so you don't need a compiler to install (once we have separate packages per platform this will be much easier), but in general it is very stable. It's exceedingly rare that an issue comes up that is caused by the watcher itself. Implementing low level stuff like this in compiled languages with deep integration into the OS provides a lot of benefits, especially in regards to performance (e.g. watching and debouncing occurs on a background thread).

@parcel/watcher is extremely widely used at this point (3M+ downloads per week on npm alone), including by VSCode, Nuxt, Nx, Parcel, etc. If there are things Vite needs in order to adopt it, I'm happy to help where I can.

@devongovett
Copy link

Update: I've released @parcel/watcher v2.2.0 which publishes individual packages per platform/architecture, and includes prebuilt binaries for a bunch more as well. See the release notes for a full list.

@bluwy
Copy link
Member

bluwy commented Jul 2, 2023

I made two branches to test it out:

Made fs.watch mainly because we need a wrapper for @parcel/watcher to work, and that wrapper can be easily applied to fs.watch too. Both branches are not optimized and failing in tests now though (pnpm test-serve).

For @parcel/watcher, I'm getting FATAL ERROR: HandleScope::HandleScope Entering the V8 API without proper locking in place. For fs.watch, the tests are hanging somewhere. Both are likely because they are creating many watcher instances to fit Vite needs, where we need an API to watch out-of-project-root paths.

@devongovett
Copy link

The Entering the V8 API without proper locking in place error appears to be due to the tests running inside worker threads. If you set the vitest threads configuration to false it does not occur. This hasn't been something we've needed yet, but I can look into it. Sorry about that!

@devongovett
Copy link

devongovett commented Jul 5, 2023

I did some work to support multiple threads over in parcel-bundler/watcher#146. There is an alpha version published if you want to try it out: v2.2.1-alpha.0. I ran the vite tests from your branch with this version and it seemed to work better. The remaining failures seem related an API change for the on method in your Watcher class that aren't accounted for in @vitejs/plugin-vue for example (causing "eventHandler is not a function" errors later). Let me know if this works better for you. I'll be doing a bit more testing on my side as well to ensure this is stable before releasing.

@bluwy
Copy link
Member

bluwy commented Jul 5, 2023

Thanks! I forgot to give an update that the threads: false tip helped. I've fixed the plugin-vue issue and pushed it to the branch. I'm not getting the V8 errors now, but there are existing HMR test fails which I haven't figure out yet.

I'm also curious on your thoughts if the Watcher class and how it calls subscribe is performant. I do wish @parcel/watcher could provide that Watcher interface directly instead (similar to chokidar), but maybe that's out of scope of the project.

@devongovett
Copy link

Yeah it's probably not the most performant. On macOS it's probably fine, but on Linux there is a pretty low inotify watcher limit and watchers are not recursive by default so we have to traverse the directory hierarchy and watch each sub-directory manually. I noticed that you're calling subscribe on a lot of overlapping directories inside the project, rather than watching once from the root of the project. This will currently result in common directories being traversed and watched more than once.

If possible I'd recommend watching from the root of the project instead of watching each directory. You could also potentially use a similar interface to what you have but keep track of all the directories that were watched and find the minimal set of common root(s) between them to actually watch. We could potentially do something like that in @parcel/watcher itself eventually, but I might even implement that as a JS wrapper anyway - not sure if there is much benefit to doing it in C++.

@bluwy
Copy link
Member

bluwy commented Jul 5, 2023

It currently also adds the project root here

watcher.add([root, ...config.configFileDependencies, config.envDir])

So the subscribe shouldn't be called too much as there's an overlap guard, unless it's out of root. In Rollup/Vite, we have a this.addWatchFile API for plugins to watch and depend on certain files, so we need a Watcher.add function to dynamically handle those. A potentially not-so performant part with this is that we can only watch directories, so addWatchFile can only watch the file's directory and indirectly watch more than it's needed.

@ArnaudBarre also had an idea that we only watch what's needed. For example, at the start we don't watch anything, but as we start to serve the app and crawl the transformed files, we add each of them to be watched. With the current Watcher class, this might not be great for perf.

@devongovett
Copy link

Apologies for the delay! I have now released @parcel/watcher v2.3.0, which includes support for worker threads, a new kqueue backend to support freebsd, and a wasm backend to support stackblitz. https://github.com/parcel-bundler/watcher/releases/tag/v2.3.0

@bluwy
Copy link
Member

bluwy commented Oct 23, 2023

I've took another stab to use @parcel/watcher (#14731). We need to keep compatibility with chokidar's API for now to not break the ecosystem, which I implemented in the PR, but it's becoming a larger undertaking than expected.

The wrapper to use @parcel/watcher is fairly big that it might be worth creating a separate library too, so for now we'll be moving this out of the Vite 5 milestone. We can definitely experiment this more in future minors (e.g. optional peer deps to enable it) and then create a migration path for the ecosystem to move to this new watcher API.

@bluwy bluwy removed this from the 5.0 milestone Oct 23, 2023
@gajus
Copy link
Contributor

gajus commented Oct 23, 2023

@bluwy If compatibility with chokidar is desirable, what was the reason for not considering Turbowatch?

We've been using it now for ~7 months without an issue.

@bluwy
Copy link
Member

bluwy commented Oct 23, 2023

Personally the package size is too big (though Vite bundle dependencies and I have not measured what the size would be). But it also depends on chokidar, and in that case it would be simpler if we use chokidar directly? It looks certainly robust but we're not going to use all of its features, and we usually try to balance the cost of functionality per package size in Vite.

@gajus
Copy link
Contributor

gajus commented Oct 23, 2023

Interesting. Was not even aware of the package size.

If zx gets updated (or if it is made a peer dependency), the size drops to 10MB.

Removing of the other dependencies though would be challenging without compromising the developer experience.

Could maybe separate the core from the binary. Happy to explore if there is interest.

@devongovett
Copy link

Please let me know what would help. We can probably put at least some of what you need in @parcel/watcher itself. I've already put in quite a bit of work to support Vite, so it would be awesome to see it through! 🙂

@bluwy
Copy link
Member

bluwy commented Oct 24, 2023

@devongovett Appreciate the work as always! I think what we need is an easy migration path to use it, since it's usually an implementation detail so we don't want it to be too breaking. A couple ideas:

  1. Create a chokidar fork with the same API but uses @parcel/watcher.
  2. Or create a new (and better) watcher interface, and Vite can easily create a chokidar compat layer.

The packages could be done by anyone though. We can then support as an optional peer dep and push it. However I'm a bit tied on bandwidth at the moment 😬


What would be nice in regards to @parcel/watcher is:

  1. Support a non-recursive mode parcel-bundler/watcher#92
  2. A way to identify if create or delete is caused by a file or directory (chokidar has this, though I don't have a large usecase for it)

Additional notes:

  1. We initially thought that @parcel/watcher was similar to chokidar, but it seems to be closer to fs.watch so it's not the anticipated amount of work we expected for the milestone. Though I tried my best attempting it.
  2. An ideal case would be to have Rollup adopt @parcel/watcher too, and I think the requirements would be the same as us.

@ArnaudBarre
Copy link
Member

ArnaudBarre commented Oct 24, 2023

From what I know of Vite HMR, I don't think 1. and 2. are needed. Plus I think that this change would be the occasion to remove the default watch of the whole working dir that then need some edge cases. This is something that I like to work on, but didn't get to it during v5 beta

@bluwy
Copy link
Member

bluwy commented Oct 25, 2023

1 and 2 is needed because we expose server.watcher. I've also thought about the "watch lazily" plan but ultimately I dont think it's feasible because:

  • Feels like a big overhead on startup as you're watching many tiny files/directories in a burst, and need tooling to consolidate and deduplicate watchers. (This is also where a chokidar like interface makes it easier to manage)
  • We cannot detect added/removed tsconfig.json within the root that would've affected .ts files. (discussed with dominik about this one)

@ghiscoding
Copy link
Contributor

Chokidar also supports glob pattern for file watch and ignored option as well but I don't see any glob support by Parcel Watcher (at least not from the docs on the readme).

To be fair though, it seems that the Chokidar author wanted to remove globs in this open Chokidar version 4 PR but I doubt it would be merge anytime soon since the PR is 1.5 year old at this point... which reminds me globs feature was removed from rimraf not long ago but quickly added back because too many users begged for the glob support :). So anyway, does Parcel Watcher support globs and if not then how would Vite go about it?

@devongovett
Copy link

Yes, glob ignores are supported. They're pretty efficient too - they get compiled to a regex and matched in the C++ layer (off main thread). https://github.com/parcel-bundler/watcher#options

@benmccann
Copy link
Collaborator

Yeah it's probably not the most performant. On macOS it's probably fine, but on Linux there is a pretty low inotify watcher limit and watchers are not recursive by default so we have to traverse the directory hierarchy and watch each sub-directory manually.

Node just made native recursive watching available on Linux. It was originally added in Node 20, but had some bugs, so you couldn't use it just yet. It seems to be working now as of Node 20.13/22.1+. Perhaps @parcel/watchers could adopt it now?

@devongovett
Copy link

We have already used the same OS APIs that node is using for years. Their implementation would be subject to the same OS limits.

@benmccann
Copy link
Collaborator

Ah, yeah. I guess you would need to implement fanotify to address this (parcel-bundler/watcher#87)

@benmccann
Copy link
Collaborator

We need to keep compatibility with chokidar's API for now to not break the ecosystem

This feels like probably too large of a barrier to me. Vite exposes chokidar's options, which means you would need to re-implement every single option that chokidar has. And it's also a bit constraining too - what if @parcel/watcher has an option that chokidar doesn't? Then how do users access it? If we're releasing a new major then I think we can break the API and have folks migrate. Especially since most users won't be setting the server.watch option anyway.

If we told people they needed to migrate from chokidar to @parcel/watch then there are some things that don't seem possible to migrate today, which is the larger concern to me. Probably the largest one I noticed is the followSymlinks option that chokidar has : https://github.com/search?q=followSymlinks+path%3Avite.config&type=code

What would be nice in regards to @parcel/watcher is:
Support a non-recursive mode parcel-bundler/watcher#92

Does chokidar support this? I didn't see that it did, but might have missed it

A way to identify if create or delete is caused by a file or directory (chokidar has this, though I don't have a large usecase for it)

I guess this refers to having add|addDir|unlink|unlinkDir events.

I found only one example of listening only to directory events: https://github.com/avisek/frontend-mentor-solutions/blob/8deb2852d58228a4dffe06d4653c8b8ceab904d0/vite.config.ts#L174

Listening only to file events happens more often, but I'm not sure if adding in directory events would harm anything in these use cases or not.

E.g. with vite-plugin-watch-and-run:
https://github.com/patsissons/movies.place/blob/822051126dc30e077bb381528f62c7e7fe9fe429/vite.config.ts#L26

And vite-plugin-virtual-mpa:
https://github.com/PhoenixIllusion/babylonjs-jolt-physics-plugin/blob/9e8980804bbea735cdadc141553b2eeb7f32cf6e/basic-examples/vite.config.ts#L29

@ghiscoding
Copy link
Contributor

Does chokidar support this? I didn't see that it did, but might have missed it

I assume you made a typo and meant @parcel/watcher instead of chokidar

@benmccann
Copy link
Collaborator

No, I actually meant chokidar. Because if chokidar doesn't support it, then we're no worse off if we switch to another library that also doesn't support it

@devongovett
Copy link

Just so it's on the record: I am very open to adding new features to @parcel/watcher to support Vite. We just need to define what those features are, and someone needs to have time to implement them.

@benmccann
Copy link
Collaborator

benmccann commented May 21, 2024

it seems that the Chokidar author wanted to remove globs in this open paulmillr/chokidar#1195 PR but I doubt it would be merge anytime soon since the PR is 1.5 year old at this point...

Someone else picked up that PR in paulmillr/chokidar#1304, which is being actively worked on, so it does look like glob support will probably be removed from chokidar soon

@benmccann
Copy link
Collaborator

I wonder in which ways @parcel/watcher is better than chokidar? It sounds like it might primarily be better for Windows users:

Parcel's watcher is much faster out of the box and additionally supports watchman as a backend, which improves support for Windows and minimizes overhead for projects already using watchman.

But is it worth adding all these native dependencies for Mac and Linux users? Why not just use the Node.js APIs on those platforms instead of needing a native distributable?

@devongovett
Copy link

devongovett commented May 22, 2024

Why not just use the Node.js APIs on those platforms instead of needing a native distributable?

same reasons you'd use chokidar: https://github.com/paulmillr/chokidar?tab=readme-ov-file#why

I wonder in which ways @parcel/watcher is better than chokidar?

Generally more efficient: It is implemented natively, runs in a background thread, avoids crawling the file system on startup (see tweet linked above from when Nuxt switched), etc. See also: this issue on vscode which led to them switching away from chokidar. microsoft/vscode#3998

@bluwy
Copy link
Member

bluwy commented May 22, 2024

@benmccann

Re chokidar options, I mean it's exposed API instead, under server.watcher where plugins could call .add() to watch additional files etc. The options can be handled like how lightningcss is integrated today, so we don't have to match the options.

Re non-recursive watching, chokidar supports this when you watch a single file, and only its directory is watched. Any recursive directories within it is not watched. This is commonly used for this.addWatchFile.


Also, (personally) @parcel/watcher's scope now is more comparable to fs.watch than chokidar. We need a higher-level interface to manage watched files. (The relationship is like svelte and sveltekit)

And with chokidar v4 coming along (could propose changes), plus node constantly improving fs.watch, maybe it still make sense to stick with chokidar?

@benmccann
Copy link
Collaborator

@devongovett what I meant is why doesn't @parcel/watcher use the Node.js APIs instead of needing a native distributable? Is there some API that Node doesn't expose that @parcel/watcher requires or something like that?

I just looked through the two open issues linked to from the issue description and they don't really relate to what file watching library we use. One is about which files we should watch and the other is about the dev server hanging mostly related to circular dependencies or maybe too many open files (but not too many watched files).

I'm wondering if anyone is even having issues with chokidar now at all or what the driving motivation for a switch would be. It would be helpful if there were some very large project we could test on where you could clearly see the difference between libraries. While some other projects have chosen @parcel/watcher, they have different usage patterns and make use of different APIs and so I don't know if people are seeing the same types of issues while using Vite. I don't really see issues clearly attributable to chokidar in the issue tracker here.

@devongovett
Copy link

devongovett commented May 22, 2024

And with chokidar v4 coming along (could propose changes), plus node constantly improving fs.watch, maybe it still make sense to stick with chokidar?

Is there info on this somewhere? The v4 branch on the repo is over 2 years old.

why doesn't @parcel/watcher use the Node.js APIs instead of needing a native distributable?

It uses low level C APIs provided by the OS that Node doesn't use or expose. That's why it's more efficient.

What's your motivation for commenting on this issue? You don't like native modules? FWIW, chokidar uses a native dependency too. Seems like you're interested in this space but if you're not having issues with chokidar then why get involved here?

I'm wondering if anyone is even having issues with chokidar now at all or what the driving motivation for a switch would be.

There are clear issues that led to this being opened in the first place, and led others like Nuxt to switch. I don't use Vite so I'm sure others can speak to this better than me. I'm mainly here to help Vite adopt if they choose to. I have already put in a significant amount of work to help with this, including implementing several new features, as you can see in the thread. So far it has been deferred, but I remain available to help if needed.

@benmccann
Copy link
Collaborator

Is there info on this somewhere? The v4 branch on the repo is over 2 years old.

There was active discussion today about releasing chokidar v4 in the next month: paulmillr/chokidar#1304

It uses low level C APIs provided by the OS that Node doesn't use or expose. That's why it's more efficient.

Any details you can share? I'm curious which situations we'd see the performance differences

What's your motivation for commenting on this issue? Seems like you're interested in this space but if you're not having issues with chokidar then why get involved here?

I'm a maintainer and want to ensure we're doing our due diligence in choosing whether to introduce breaking changes, native dependencies, etc.

You don't like native modules? FWIW, chokidar uses a native dependency too.

I think that they can at times introduce an extra layer of complications. I know I've hit issues with sharp's on numerous occasions, but esbuild's have never caused much of an issue. I'm not really sure why they've caused me headaches in one situation and not the other, but it does seem like it would be nice to avoid if possible. We do use native dependencies already, so I'm not saying we can't use native dependencies, but I was interested to understand why they help since it seems like it's pretty much the same underlying API used (e.g. inotify) in either case

FWIW, chokidar uses a native dependency too.

I guess you're referring to fsevents? It's an optional dependency only used on MacOS and has been removed entirely in chokidar v4 now that updates in the latest versions of Node.js make it obsolete

I have already put in a significant amount of work to help with this, including implementing several new features

I definitely appreciate the willingness to help!! And to be clear I'm just one of many maintainers and not a core maintainer, so wouldn't want you implementing anything based off anything I've raised here. I think the Vite team as a whole should discuss whether we'd want to migrate and what the blockers would be so that we don't cause you any unnecessary work.

@devongovett
Copy link

Ok, here are the main things I am aware of in regards to performance:

  1. Recursive file watching is supported on all platforms natively. If crawling needs to be done (e.g. on Linux where inotify is not recursive at an OS level), it is done on a background thread. This used to not be supported by Node on Linux at all, but they recently added support. However, like chokidar before, it is implemented in JavaScript on the main thread, and does not support ignore paths, so it is unlikely to be much more performant than chokidar. This likely has a huge impact on startup times on these platforms.
  2. Events are batched in a background thread and delivered to the JS thread all at once rather than per file/event. This is especially useful for large changes like switching git branches or running npm install. In these scenarios, there are often hundreds of thousands of events emitted by the operating system, some of which are irrelevant or temporary. For example, there will be a separate event for the creation of each file, then writing to it, updating the metadata (e.g. mtime, size), etc. Sometimes temporary files are created and then renamed or deleted as part of these processes. @parcel/watcher will batch all of these events together and only report the final state to the JS thread, and in addition batch together events from multiple files into one callback. This means the JS thread only needs to worry about the changes that actually matter rather than intermediary states, and doesn't get overwhelmed with unnecessary events that invalidate the build many times. fs.watch always calls the event callback immediately for each individual event of each individual file.
  3. Ignore lists are processed in the native layer. These can be directories to ignore like .git, or globs like **/dist/**. This is especially efficient on some operating systems that provide a native way to ignore paths, in which case events for irrelevant files are never even delivered to the process. In other cases, @parcel/watcher can efficiently process the ignores in its background thread, and the JS thread doesn't need to worry about these. fs.watch doesn't have an option for ignore paths or globs at all.
  4. On OSes where @parcel/watcher needs to crawl the file system to create watchers (e.g. Linux/BSD), it can avoid even creating watchers for ignored directories entirely. This helps with common problems like running out of file descriptors, as well as startup performance. fs.watch and anything built on it like chokidar, has no way of natively ignoring files like this. It is forced to handle all events and throw them out after, resulting in worse startup performance and file descriptor errors.
  5. Multiple watchers for the same directory are deduplicated and only result in a single native watcher, reducing startup cost and consumed file descriptors.
  6. Watchman is used if installed. This improves performance especially on linux because watchman runs as a daemon in the background, so it doesn't need to re-crawl the file system every time your process restarts.

In terms of reliability, @parcel/watcher fixes many of the caveats in the node docs for fs.watch, similar to chokidar. It ensures that filenames are always reported, supports recursive watchers on all platforms, attempts to determine the real type of change that occurred when files are renamed, etc.

I am aware that there have been some improvements to fs.watch lately and that's great, but it is quite a bit lower level (basically just delivers OS events directly) so a library on top of it like chokidar or a replacement like @parcel/watcher is probably still needed to get good reliability and consistency across platforms. @parcel/watcher has the additional benefit of using OS APIs directly rather than being built on top of fs.watch, so it can achieve better performance for the reasons I outlined above as well.

@benmccann
Copy link
Collaborator

Thank you @devongovett!! This is really great information! (You should add it to the @parcel/watcher readme or docs as it's really great advertising 😄)

@paulmillr
Copy link

has the additional benefit of using OS APIs

with a disadvantage of being a native module which doesn't run everywhere

@devongovett
Copy link

We've worked pretty hard to support as many platforms as possible. Is there one missing that you need?

@paulmillr
Copy link

not really

it’s that I maintain fsevents and through the years and nodejs updates some bug (incl compilation bug) somewhere will arise. so it needs to be constantly updated and supported.

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

10 participants