Implements custom recursive watch for linux #39
Conversation
servor.js
Outdated
| @@ -132,6 +139,6 @@ module.exports = async ({ | |||
| root, | |||
| protocol, | |||
| port, | |||
| ips | |||
| ips, | |||
There was a problem hiding this comment.
@lukejacksonn do we need this dangling comma?
There was a problem hiding this comment.
perhaps this is an editor config? there are a lot of dangling commas in the code now. 🙈
There was a problem hiding this comment.
Yeh I realised.. this is generally my personal preference (for browser based projects at least).. when was this supported in node do you know? Is it likely to cause issues?
There was a problem hiding this comment.
I've just reverted that change.. it didn't belong in this PR 😅 now the servor.js diff is all relevant to the fix!
lukejacksonn
force-pushed
the
custom-recursive-watch
branch
from
bfb3f80
to
368a5df
Compare
servor.js
Outdated
| const watch = (path, cb) => { | ||
| if (fs.statSync(path).isDirectory()) { | ||
| fs.watch(path, cb); | ||
| fs.readdirSync(path).forEach(entry => watch(`${path}/${entry}`, cb)); |
There was a problem hiding this comment.
I feel like this might become too expensive. I use servor at the root of repository, which has lot of directories in .git, node_modules, etc.
maybe, we should consider being able to configure what to watch?, something like:
servor --watch dist --watch src <...other options>also this way the watch/reload become less aggressive/chatty...
There was a problem hiding this comment.
I have considered this too! I'm not sure exactly how expensive this is.. but apparently the native { recursive: true } is much more efficient, to the point where you can do:
// watch the whole disk
fs.watch('/', { recursive: true })Source: https://www.npmjs.com/package/node-watch
So I am going to put a ternary in that will use this for linux and native recursive fs.watch on windows and mac (then we shouldn't see any regression on existing perf).
Regards you wanting to watch only certain things I'm playing with the idea of a config file to put all these harder to define inline kind of options in. As I have mentioned before I want to keep the CLI and node API as simple as possible.
There was a problem hiding this comment.
One more alternative you might want to consider:
Although this involves much more complicated options forwarding through CLI.
There was a problem hiding this comment.
I'd rather keep options to to a minimum as branching like this creates more and more cases that require testing etc. I have been on vacation the past week and only just getting getting a chance to go through GitHub notifications. Will push the native/custom watch switch change asap. Thanks for your patience @thisguychris and @osdevisnot 🙇
There was a problem hiding this comment.
I have pushed the change now so @osdevisnot you should not notice any difference in the way servor works and @thisguychris it should still work for you on linux! 🤞
There was a problem hiding this comment.
@lukejacksonn confirming that it works, more details on #36
lukejacksonn
force-pushed
the
custom-recursive-watch
branch
from
50f2bfd
to
7c7b036
Compare
Hopefully fixes #36 (haven't got a linux machine to test it on).