Moving from vinyl-fs watch (gaze) to chokidar, fixes #1247 #1259
Conversation
| var watcher = gulp.watch(relFile, {cwd: cwd}) | ||
| .on('change', function(filepath) { | ||
| should.exist(filepath); | ||
| path.resolve(cwd, filepath).should.equal(path.resolve(tempFile)); |
There was a problem hiding this comment.
This one was particular tricky, as chokidar spits out relative file paths instead of absolute ones.
There was a problem hiding this comment.
Should be based on the input. If watching a relative path, events are emitted with matching relative paths. If you do path.resolve(relFile) at the top you should get absolute paths with the events.
Not at all. Happy to have some help and to see this moving forward without me being the bottleneck. |
|
|
||
| #### fn | ||
| Type: `Function` | ||
|
|
||
| An [async](#async-support) function to run when a file changes. | ||
|
|
||
| `gulp.watch` returns an `EventEmitter` object which emits `change` events with | ||
| the [gaze] `event`: | ||
| `gulp.watch` returns an `FSWatcher` object that already listens to the `change` |
There was a problem hiding this comment.
Suggested edit:
returns a wrapped [chokidar] FSWatcher object. If provided, the callback will be triggered upon any add, change, or unlink event. Listeners can also be set directly for any of [chokidar]'s events.
|
So long as you all are comfortable with the resulting API changes, then this implementation LGTM. |
|
Oh, btw, I should be able to get chokidar 1.1.0 released next week. This will include fsevents 1.0.0, which now uses node-pre-gyp to make typical installs on Mac much faster and more stable. |
| console.log('File ' + event.path + ' was ' + event.type); | ||
| watcher.on('change', function(path, stats) { | ||
| console.log('File ' + path + ' was changed'); | ||
| if (stats) { |
There was a problem hiding this comment.
can this be updated so stats is always present? don't like having this be optional
There was a problem hiding this comment.
It would be a perf hit, and most users don't need/use it. Existing interface doesn't provide a stat object at all afaik, perhaps it just shouldn't be passed through here.
Or perhaps only provide it if the user specified alwaysStat
There was a problem hiding this comment.
@es128 maybe just remove it from the example then, it's confusing
|
left comments |
|
Just want to let you know that I'll look into everything once I'm back from Italy ;-) (Wednesday the earliest) |
|
@ddprrt cool. So if I have some time tonight or tomorrow I may also take a shot at addressing the feedback, starting with your commit as a basis. I believe it's only documentation changes left to do. Enjoy the rest of your trip! |
|
I'm going to be taking point on making sure this gets finished and merged FYI. Ping me when the changes have been pushed to the fork since GH doesn't notify on new commits. |
Moving from vinyl-fs watch (gaze) to chokidar, fixes #1247
|
You want a new PR with docs updates? |
|
Merged this so we can get the functionality in, any docs stuff is a part of this #1281 |
|
We still need to explicitly bump chokidar to 1.1 in the deps so everyone gets the pre-gyp |
|
Not strictly necessary, but sure. |
|
@phated uses a caret range but I updated it anyways |
I gave integrating chokidar a shot. I hope you don't mind @es128. I see it as some exercise for me, so if you have any better solution, I don't mind having this PR going down ;-)
Naturally, I felt really uncomfortable changing tests ;-) But due to chokidar's different API, I had to. The changes were rather small, though. I also updated the recipes and docs.
Looking forward to your feedback
Fixes #1247