Workaround for the "Windows rough edge" regarding the deletion of directories #112
Conversation
…or when remove a directory
| // Workaround for the "Windows rough edge" regarding the deletion of directories | ||
| // (https://github.com/joyent/node/issues/4337) | ||
| if (platform === 'win32') { | ||
| watcher.on("error", function (err) { |
| // Workaround for the "Windows rough edge" regarding the deletion of directories | ||
| // (https://github.com/joyent/node/issues/4337) | ||
| if (platform === 'win32') { | ||
| watcher.on("error", function (err) { |
There was a problem hiding this comment.
maybe we can check for error code here and execute fs.exists only for some errors?
There was a problem hiding this comment.
The error object you get is { [Error: watch EPERM] code: 'EPERM', errno: 'EPERM', syscall: 'watch' }. It's not very meaningful, but we could check the error code to reduce the impact of the workaround further. On the other hand it can't hurt to ignore errors for non existing items. I'll leave the decision to you.
There was a problem hiding this comment.
let's limit this to EPERM for now. We can expand it later.
|
@paulmillr are you satisfied with the current solution? If so then it would be nice if you would merge and release the change. I am currently replacing fs-watch-tree with chokidar in buster.js and i need this fix therefor. |
|
@dwittner it would probably be a good idea to squash the additional commits that were added subsequent to code review to get the PR ready for merge |
|
I have improved your algorithm — now behaves as before for real |
|
@paulmillr, when will the fix approximately be published? |
|
@paulmillr, unfortunately the improvement doesn't seem to work, i still get the error "Error: watch EPERM". Do i do something wrong? |
|
are you using github version? do you actually have right permissions? |
|
I use npm version 0.8.4, which seems to contain the fix for the windows rough edge. |
|
well that's weird then. no idea why this happens, sorry |
|
Now your code raises an exception: index.js:289 ... because it's not passed as Parameter |
|
@paulmillr, could you please fix your modification to fix the ReferenceError? |
In the issue joyent/node#4337 it is recommended to add an error handler to the Windows watchers and to ignore the error. That's what this fix does. It adds an error handler, if the platform is "win32". To reduce the impact of the workaround, the error is only ignored, if the corresponding item doesn't exist.