-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[Packager] Switch file watcher to Chokidar #628
Comments
He has the point. If chokidar has serious problems, why doesn't anyone report them? Are you absolutely sure that sane (which uses child_process) would handle large codebases without issues? |
We're primarily interested using watchman. Sane is just use for a fallback. Watchman is battle tested on facebook infrastructure, developer laptops and many open source users. I'm still open to exploring that option but I believe that watchman is a much better technology.
That's great. But by the same standards watchman is a much popular open source project than chokidar, it has 2.5k github stars while chokidar has 700 :)
Again, watchman is also battle tested 36 releases :)
Yes, because you use a native fsevents module. |
I did report them in the past. It was incredibly slow to use on our codebase, I needed a ready event which I sent a pull request for that wasn't accepted. Even with the ready event it was no match to watchman's instant daemon model. Also "child_process" is dishonest. We only |
Would be great to go full NPM without
Right. Sorry for that, the import confused me.
|
@amasad Do you perceive that there would be value in adding a watchman option to chokidar for users willing to do the separate non-npm install? If we were to move in that direction, is there potential for consolidating efforts toward a single library, or are there any other concerns/shortcomings with the recent versions of chokidar that would prevent you from considering this? |
Maybe @wez can think of a way this could work. But the way watchman works -- and what makes it fast and reliable -- is that it's a daemon.
So this shells out to get the socket name and then just uses unix sockets to connect to watchman.
Yes, this sounds great. If chokidar has all the backends/fallbacks that sane has then there is no reason for sane to exist. |
Well let me just finish pushing 1.0.0 out the door (should be next week), but after that I'd be happy to collaborate on giving this idea a try. |
Here is the watching backends that would be great to support in this consolidated effort:
Thoughts? |
By the way, one of the remaining pain points remaining on using chokidar with extremely large file trees isn't the actual watching (unless using polling) - it's the need to scan the entire tree at instantiation as a basis for normalizing events. Currently use readdirp for this, have been thinking about submitting a patch there to create a "fast mode" option that avoids getting a stat for each file. |
@amasad sounds great |
Well yeah - chokidar already has great support for 2-4 on your list. The effort would be to add watchman, and preferably detect its presence to have it kick in by default when available - otherwise fall back to existing defaults. |
Also, regarding polling, see paulmillr/chokidar#242. Been mulling over how to solve this as well. Want to cover as many cases as possible where we can get it to "just work" without any environment-specific settings needed. |
I'd be happy to help you figure out how to consume watchman from chokidar. For large trees and multiple tools wanting to consume file notifications, using watchman as a consolidation point will reduce overall resource utilization and avoid node-process startup time overheads (particularly if you restart your node process). |
@es128 cool! It seems like chokidar has really improved in the year that I hadn't looked. Let's make this happen. cc @stefanpenner any concerns? |
Strongly recommend that you use the new (just landed) watch-project command for your integration to get the best consolidation: |
haha yeah well I've been putting a lot of effort into it since around October |
@wez Cool - thanks for the tips |
Without digging in further, the only superficial concern is the Maybe node-gyp / chokidar have improved dramatically since I last used them, and my concerns are now pointless. I'll try to find some time in the next few days to play, and come up with some concrete feedback. |
@stefanpenner It's worth noting that fsevents is a optional dependency for chokidar. It continues to function adequately without it. And lots of people are using this today, very few have problems or concerns. There's also an effort underway in the fsevents project to speed up installs using node-pre-gyp to distribute pre-compiled binaries. |
If it fails to build, it does create an extremely noisy/confusing experience.
It is a pretty disheartening experience, for an optional piece of infrastructure to draw so much attention to itself, when it fails. |
Yes I agree. And I deal with it when people come asking for help. At this point the answers are easily google-able. Still, wish npm handled it better. |
this is sub-optimal, it reflects poorly on us (tooling maintainers) and increases pain and noise in our issue trackers. It is our responsibility to provide frictionless experiences, so developers can just do their actual work. Theoretical ideal resolutions:
although these solutions will likely not satisfy short-term use-cases, it would be great to try and get the ball rolling so someday we can just have nice things. |
Didn't anyone check to see if things got better in node v0.12 or iojs? |
Yes. And it didn't. Or, maybe better. But still far from adequate. |
is there a relevant issue open on libuv? |
Progress is being made all the time, but there will always be issues and more to improve/solve. |
There's plenty of discussions I've seen in the node and libuv issue trackers that tend to culminate in "we're exposing what the OS provides, nothing more we can do". Here's a more recent one in io.js that is somewhat relevant: nodejs/node#375 And there's actually a reasonable point they're making there for many circumstances. For instance, editors often cause a lot of churn when they save files, and I'm not sure it's core's job to normalize that kind of thing. But to provide a smooth experience in tooling, the normalization is necessary. So there will likely remain an ongoing place for higher-level userspace modules that do this sort of work. |
Ya the editor induced churn is painful. Maybe watchman itself we can push through the ecosystem to help facilitate better experiences. I suspect it fairly unlikely our os vendors will be likely to improve in the short term, but maybe it's worth dreaming? |
This long running chokidar issue was brought to my attention. I'm going to leave a mention of it here for posterity paulmillr/chokidar#219 |
In my experience, the watchman process is the most stable form of watching I have yet to encounter. I re-visited the c extension problem and the issue persists. New users are extremely confused, often believing the entire attempt failed or at best assuming a quality issue in the tool they use. One downside though, is that if the c-extension fails, the resulting watching experience may be as broken as the c-extension failing implied :( 2015 and we have to resort to hacks to get meaningful information change events form the FS. What a sad state of affairs. |
@stefanpenner the npm UX issue is different from the one @amasad is referring to. There is an issue in fsevents when it's bombarded with simultaneous events in some specific circumstances that sometimes causes processes to crash with a segfault - notably doing Nonetheless, as has been discussed in this thread, looking forward to adding a watchman interface to chokidar which would provide another option for mediating this and other problems that still exist with the lower-level watch methods currently being relied upon.
I disagree with this. In this case it falls back to the same code that works very well on Linux and Windows. It's not as good as it would be with |
Yes. I am raising an additional concern. |
For the most part, this impacts Mac users who haven't installed XCode (or the command-line tools). Is that who you're focusing on by raising the concern? Or is there another persona I'm unaware of that struggles with this? At least as much savvy is required to go out and install watchman as to resolve the XCode thing. Which is why facebook/watchman#91 would be nice. Also, fsevents/fsevents#59 is in progress which may largely alleviate this problem as it relates to chokidar/fsevents. |
There is another cohort, one with Xcode installed but with a broken compiler tool chain. This is sadly common with those with enough context with the tech to be dangerous but insufficient context or time to resolve. This can easily occur when upgrading An OS, or botching something as simple as env $PATH. Although it would be nice to re-delegate this issue back to the user in question. Our goal should be to reduce complexity/ friction/ noise not to compound it unless absolutely needed. I realize I am likely coming off as a pain, but my motivations are really just to pave the pain points I observe from my users. |
Absolutely, to whatever extent is possible |
Going to close because it's not really actionable (we can continue to discuss). Let's revisit if once we have a plan to address the issues above. |
Hey. You are currently using sane for file watching. How about using chokidar?
I know that Sane is made by facebook dude, but how about trying more stable & robust solution?
The text was updated successfully, but these errors were encountered: