fs: remove fs.watchFile(), IOWatcher

[bnoordhuis]

IOWatcher should be non-controversial, nothing is using it. I could patch it up to work with libuv's new poll API but I don't think there's much point.

fs.watchFile() is currently broken in master: it depends on libev behavior that's been supplanted by libuv. There's no fix except writing our own stat poller (possibly in JS). I move to remove it.

/cc @isaacs, @piscisaureus

[isaacs]

How hard would it be to write a stat poller as a userland module? It seems like something that could be done pretty easily in JS, actually.

[bnoordhuis]

Not hard. A timer and fs.stat() / fs.fstat() is all you need. Thread pool contention is a possible issue but we need to address that anyway.

[defunctzombie]

can fs.watch still watch a file? What was the purpose of fs.watchFile over fs.watch? I guess the stat object is a nice to have. Wasn't there 3rd party modules that do inotify, etc for people that care about that?

[bnoordhuis]

can fs.watch still watch a file?

Yes, fs.watch() still works (and is the preferred API anyway).

What was the purpose of fs.watchFile over fs.watch?

Nothing, it predates fs.watch(). We added fs.watch() because fs.watchFile() is reliable, fast nor scalable.

[domenic]

I love that Node is still willing to make changes like this (i.e. doesn't keep old APIs around just for backward compat). <3

[isaacs]

@domenic Well, we DO care about backwards compatibility.

We ought to deprecate fs.watchFile in 0.9 and replace it with a stat poller.

[agnat]

Well, the mdns addon relies on IOWatcher ever since. After pondering it for some time I'd really appreciate a uv_poll based replacement on the node side. I think it is a very useful utility to wrap third party libraries. It also will be much easier to maintain if it is part of node and not an external component. It probably should be renamed to SocketWatcher though, because AFAIK the windows implementation works with sockets only.

It would be awesome if you guys could add such a thing at some point in 0.9.

[TooTallNate]

It also will be much easier to maintain if it is part of node and not an external component.

I humbly disagree. If it works fine as an external module, then I see no reason for it to be in core. It's just more maintenance for the core team to deal with in the future.

[andrewrk]

I use fs.watchFile over fs.watch because with fs.watch I end up hitting the system's limit for number of open file descriptors. I still agree with this decision, however. It makes sense for whatever ends up providing the fs.watchFile implementation to be in userland.

[andrewrk]

Also, with fs.watch, often I find that if you try to read from a file which has just been updated, for example with vim, the file happens to be temporarily completely missing! Not sure whether this is a race condition or what, but I've found fs.watchFile to be less error prone.

[medikoo]

Just adding few notes: I think It's important to have alternative that does not eat file descriptors and also works in not persistent way.

Currently I don't see a way how to build (with plain JS) stat poller that would also be not persistent, so hearing that fs.watchFile (which gives that possibility) will be deprecated, sounds controversial.

[andrewrk]

@medikoo what exactly do you mean by persistent? I don't follow.

[medikoo]

@superjoe30 see persistent option http://nodejs.org/api/all.html#all_fs_watchfile_filename_options_listener it is better documented under fs.watch.

persisent: false just means that initialized watchers doesn't block natural exit of your process, in many cases it's exactly what you expect

[agnat]

In general I agree with @TooTallNate. However, this one is a little bit different. The code wrapped by IOWatcher (or SocketWatcher) resides in libuv which is updated by the node core team. It also has very little functional code. It is only a wrapper for uv_poll_t plus one or two callbacks. Any future changes will most likely be structural (ref counting, &c.). This kind of stuff can be done much quicker by the core team than by an external developer. An external developer (me) would need to watch every node release and see if things are still working. If not I'd have to take a look at e.g. tcp_wrap.cc, find out what changed and apply the exact same changes to poll_wrap.cc. Sounds impractical to me...

Loosing IOWatcher will most likely be fatal for the mdns addon. It is around since node 0.2 and required almost no maintenance because it is well designed. Without IOWatcher it will turn into a maintenance nightmare.

Although IOWatcher has no clients in node, it still is an invaluable tool to wrap third party libraries. It applies to all libraries that require the user to setup their own poll() or select() loop. So, it would be awesome if @bnoordhuis could patch it up to use the uv poll API.

[bnoordhuis]

Closing. IOWatcher got removed in 2c5828b, fs.watchFile() is here to stay.