Add descendent files to watched files #1745
Conversation
|
Thanks for the PR! Could you add a test in https://github.com/sass/node-sass/blob/master/test/cli.js |
|
@nschonni That's great! Thanks for having a look at the PR. I'd love to write tests for this but I running the tests of Haven't had any luck in getting watch mode to work either because of a failing binary detection. It defaults to some weird values: /dev/github/node-sass/vendor/bar-foo-baz/binding.node # <- bar-foo-baz? |
| // Add children to watcher | ||
| graph.visitDescendents(file, function(child) { | ||
| gaze.add(child); | ||
| files.push(child); |
There was a problem hiding this comment.
You don't want to compile the decedents of the changed file. Please remove this.
There was a problem hiding this comment.
Thanks for the feedback! You're right, that isn't necessary at all. Will fix that tomorrow
|
You can use the |
|
@marvinhagemeister do you want to finish up the review, or maybe turn on collaborator contributions so it can get merged? |
marvinhagemeister
force-pushed
the
fix_watch
branch
from
c1898f6 to
22ff385
Compare
|
@nschonni Sorry for not getting back any sooner, life got a bit hectic. Just updated the PR with the suggested change. Thanks for having another look. |
marvinhagemeister
force-pushed
the
fix_watch
branch
from
22ff385 to
b32bcc5
Compare
marvinhagemeister
force-pushed
the
fix_watch
branch
from
b32bcc5 to
71b1446
Compare
|
Thanks @marvinhagemeister! |
|
Awesome! Thanks for merging! |
Watching for changes on files with lots of `@import`s had a significant regression in responsiveness in sass#1745. The regression was caused by calling `gaze.add` unnecessarily. We only need to call `gaze.add` on files that aren't currently being watched. At the time I confirmed that calling `gaze.add` in files that were being watched wouldn't result in a leak or multiple events being fired. I however assumed calling `gaze.add` for already watched files would be very cheap. Fixes sass#1869
Watching for changes on files with lots of `@import`s had a significant regression in responsiveness in sass#1745. The regression was caused by calling `gaze.add` unnecessarily. We only need to call `gaze.add` on files that aren't currently being watched. At the time I confirmed that calling `gaze.add` in files that were being watched wouldn't result in a leak or multiple events being fired. I however assumed calling `gaze.add` for already watched files would be very cheap. Fixes sass#1869
Fixes #1731