Bundle fsevents in Node.js core

[mcollina]

One of the must-have modules in the ecosystem is https://www.npmjs.com/package/fsevents. fsevents is needed if you need to watch files on Mac OS X, as our current file watching strategy is inadeguate and unreliable.

Should we ship fsevents implementation in Node.js core?

cc @pipobscure

[MylesBorins]

I believe the bits that are done byfsevents make the most sense in libuv, but the team didn't seem to agree when I opened an issue

https://github.com/libuv/libuv/issues/2096

Does the OSX specific implementation belong in core?

[mcollina]

Reading on that topic, is the node core implementation powerful enough already? In other terms, is fsevents now redundant?

[refack]

In other terms, is fsevents now redundant?

It does seem so. To some level... Ref: https://mobile.twitter.com/refack/status/1073685596298772483 tl;dr the chokidar test suite passes almost all test with native node@12.

[mcollina]

@refack which tests are not passing? It could be a good goal to have chokidar without fsevents in Node 12.

[pipobscure]

Chokidar is code by evolution and could benefit from cleanup & refactor. As part of that chokidar could be subsumed into core.

FSEvents was also code by evolution. Except it has recently benefited from a refactor & cleanup, which has significantly simplyfies the API & the implementation.

Either or both could be in core. But I would not put in either.

—-

Now as to the question of “is fs.watch powerful enough now”.

There’s orders of magnitude difference in performance between fsevents and fs.watch even now.

Due to loads of improvements there’s nothing that can’t be done slowly using fs.watch.

[mcollina]

Either or both could be in core. But I would not put in either.

One of the major use cases for Node.js is to support the tools that are used to build modern web apps. In a significant amount of use cases, this includes watching for changes in the filesystem. As things stand now, the Node.js runtime was not able to provide a performant implementation on Mac OS X. fsevents did.

I'd like to explore a scenario where such a critical functionality would not require a native addon. Native addons could pose a security risk in a lot of cases, and avoiding them altogether is a last resort option. I was recently discussing if it was feasibile to add security policies to Node.js - the first thing that would not be possible to support are native addons.

I think it's a good question to ask. Can you articulate on why you would not put in either?

[pipobscure]

I guess we come at it from a different perspective. My main use-case has always been micro-service apis. Most of those run on anything but MacOs.

If I were to share your perspective and assume it to be the majority use-case then yes, it should go into core.

Whatever the conclusion. I’d be more than happy to help migrate fsevents into core if that’s wanted. By now I’m quite comfortable with the dichotomy between OSX run-loops and libuv run-loops required to make this work. It’s a bit messy though.

And that might be the biggest reason why I wouldn’t put it into core. It’s fairly messy. And applies only to a single OS. Is it worth “polluting” core with that?

[ljharb]

There’s tons of “pollution” in core to handle windows, which is just one OS. I’d say the real question is, is it worth not meeting the needs of an entire supported OS in order to meet some kind of “cleanliness” bar?

[boneskull]

Is it worth “polluting” core with that?

unequivocally, YES