Skip to content

Commit

Permalink
CLI: Re-implement run.watch() with 'node-watch' instead of 'sane'
Browse files Browse the repository at this point in the history
Similar to the previous commit, I considered a simple approach
of our own based on the built-in Node.js API.

The fs.watch() API is actually quite good in Node 6+ (certainly
much better than it used to be). But, there are two notable
issues that I think we should care about:

1. Its "recursive" feature is lacking on Linux (only stable
   on macOS and Windows).
2. Its ability to distinguish between create, update and remove
   events isn't very good.

This last point would be fairly easily to do on the consumer
side with a quick fs.stat() call. We could even omit it entirely
given we only use it for one word (a verb) in the CLI output.
But the first point (recursion) is slightly more involved than
I'd like to maintain locally.

All the reviewed packages essentially handle this the same way.
They create a non-recursive fs.watch() for each sub directory found
on the system, track them in an object. Then, start new ones as
needed when new directories are created, and stop old one when
directories are removed. That's about 100 lines of simple code.

Where they differ is:
* How many extra features they provide.
* How many simple functions for non-critical code are delegated
  to other packages.
* Whether they use native "recursive" when available (on macOS/Windows).
  privide in addition to that

I'm proposing we go with node-watch. This package is well-maintained,
tested with the latest Node versions, optimised for Node 6+, and
provides no additional features, and is dependency-free.

> [email protected] (current)
>   Dependencies: ⚠️
>     119 packages.
>     (concerning in terms of dicipline and security)
>   License: ✅
>     MIT.
>   Supported: ✅
>     2018 saw 25 commits, 8 contributors, 2 major releases.
>   Modern: *️⃣
>     Requires Node 6+, but not yet tested on Node 10.

> [email protected]
>   Dependencies: *️⃣
>     14 packages.
>     (okay, but could be better.)
>   License: ✅
>     MIT.
>   Supported: ✅
>     2018 saw 22 commits, 6 contributors, 1 release.
>   Modern: ✅
>     Requires Node 4+, tested with Node 10.

> [email protected]:
>   Dependencies: ✅
>     3 packages.
>   License: ✅
>     Apache-2.0.
>   Supported: ⚠️
>     2018 saw no commits or releases.
>   Modern: ⚠️
>    Still supports Node 0.1. No visible CI or commit activity
>    indicating testing with recent Node releases.

> [email protected]:
>   Dependencies: ✅
>     1 package (dependency-free).
>   License: ✅
>     MIT.
>   Supported: ✅
>     2018 saw 22 commits, 3 contributors, 3 minor releases.
>   Modern: ✅
>     Requires Node 6+, tested with Node 10.
>     Uses native fs.watch/recursive support where available.

> [email protected]:
>   Dependencies: ✅
>     2 packages (1 dependency).
>   License: ✅
>     MIT.
>   Supported: ⚠️
>     2018 saw no commits or releases.
>   Modern: ⚠️
>     Last tested with Node 0.10.
>     Uses per-file watching and polling for all platforms.

Fixes #1342.
  • Loading branch information
Krinkle committed Dec 28, 2018
1 parent 58fee14 commit 94fa19f
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 780 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"commander": "2.12.2",
"js-reporters": "1.2.1",
"resolve": "1.5.0",
"sane": "^4.0.0",
"node-watch": "0.5.9",
"minimatch": "3.0.4"
},
"devDependencies": {
Expand Down
37 changes: 20 additions & 17 deletions src/cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,27 +141,24 @@ run.abort = function( callback ) {
}
};

function watcherEvent( event, args ) {
return ( file ) => {
console.log( `File ${event}: ${file}` );
run.restart( args );
};
}

run.watch = function watch() {
const sane = require( "sane" );
const watch = require( "node-watch" );
const args = Array.prototype.slice.call( arguments );

const watcher = sane( process.cwd(), {
globs: [ "**/*.js" ],
ignored: IGNORED_GLOBS
const baseDir = process.cwd();

const watcher = watch( baseDir, {
persistent: true,
recursive: true,
delay: 0,
filter: ( fullpath ) => {
return !/\/node_modules\//.test( fullpath ) &&
/\.js$/.test( fullpath );
}
}, ( event, fullpath ) => {
console.log( `File ${event}: ${path.relative( baseDir, fullpath )}` );
run.restart( args );
} );

watcher.on( "ready", () => run.apply( null, args ) );
watcher.on( "change", watcherEvent( "changed", args ) );
watcher.on( "add", watcherEvent( "added", args ) );
watcher.on( "delete", watcherEvent( "removed", args ) );

function stop() {
console.log( "Stopping QUnit..." );

Expand All @@ -173,6 +170,12 @@ run.watch = function watch() {

process.on( "SIGTERM", stop );
process.on( "SIGINT", stop );

// initial run must be delayed by at least 200ms
// https://github.com/yuanchuan/node-watch/issues/71
setTimeout( () => {
run.apply( null, args );
}, 210 );
};

module.exports = run;
14 changes: 7 additions & 7 deletions test/cli/fixtures/expected/watch-tap-outputs.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ ok 1 foo
# skip 0
# todo 0
# fail 0
File changed: watching/foo.js
File update: watching/foo.js
Restarting...
TAP version 13
ok 1 bar
Expand All @@ -33,7 +33,7 @@ ok 1 foo
# skip 0
# todo 0
# fail 0
File added: watching/bar.js
File update: watching/bar.js
Restarting...
TAP version 13
ok 1 bar
Expand All @@ -53,7 +53,7 @@ ok 2 foo
# skip 0
# todo 0
# fail 0
File removed: watching/bar.js
File remove: watching/bar.js
Restarting...
TAP version 13
ok 1 foo
Expand All @@ -65,7 +65,7 @@ ok 1 foo
Stopping QUnit...`,

"change-file-mid-run": `TAP version 13
File changed: watching/bar.js
File update: watching/bar.js
Finishing current test and restarting...
ok 1 Foo > one
1..2
Expand All @@ -90,8 +90,8 @@ ok 1 Module > Test
# skip 0
# todo 0
# fail 0
File changed: watching/tests/foo.js
File added: watching/bar.js
File update: watching/tests/foo.js
File update: watching/bar.js
Restarting...
TAP version 13
ok 1 Module > Test
Expand All @@ -100,7 +100,7 @@ ok 1 Module > Test
# skip 0
# todo 0
# fail 0
File changed: watching/bar.js
File update: watching/bar.js
Restarting...
TAP version 13
ok 1 Module > Test
Expand Down
Loading

0 comments on commit 94fa19f

Please sign in to comment.