Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Migrate to a better file watcher #636

Open
paulmillr opened this issue Jan 22, 2015 · 15 comments
Open

Migrate to a better file watcher #636

paulmillr opened this issue Jan 22, 2015 · 15 comments

Comments

@paulmillr
Copy link

Hey. Your file watcher (gaze) has 53 opened issues. Many bugs on various platforms.

Can we make a pull request that migrates the watching to chokidar? Chokidar is ultra-stable and is used in many popular projects.

@kevva
Copy link
Member

kevva commented Jan 22, 2015

+1, I basically chose gaze because it's widely used in gulp and seems to work there. But I'm all for a change.

@paulmillr
Copy link
Author

Gulp is switching to chokidar ;) gulpjs/glob-watcher#13

@am11
Copy link
Contributor

am11 commented Jan 23, 2015

👍 Lets make it a post-v2 (stable) change.

BTW, nice name for a watcher library: Chokidar (watchman in my lang) :p

@sindresorhus
Copy link
Contributor

👍

@am11 am11 added this to the v2.1 milestone Jan 24, 2015
@callumlocke
Copy link

FWIW, I've worked a lot with gaze, chokidar and sane. sane is the newest and least well known, but it is by far the most reliable in my experience. And I think it has the most sensible API. Worth a look.

@am11
Copy link
Contributor

am11 commented Jan 29, 2015

@callumlocke, thanks for the suggestion. Will take a look.

@am11 am11 added v2.1 and removed v2.1 labels Feb 12, 2015
@am11 am11 modified the milestones: Unknown, 3.0 Mar 23, 2015
@jareware
Copy link

👍 would really like this!

@villelahdenvuo
Copy link

👍 Gaze is using ancient deprecated dependencies.

@saper
Copy link
Member

saper commented Feb 14, 2016

Some help is needed to fix the watcher tests, we had to disable them because the way the CLI was called didn't really work.

@adamreisnz
Copy link

+1 to swtich to chokidar, or at least update the gaze dependency, as the current version used by node-sass contains very outdated deps:

├─┬ gulp-sass@2.2.0
│ └─┬ node-sass@3.4.2
│   ├─┬ gaze@0.5.2
│   │ └─┬ globule@0.1.0
│   │   └── lodash@1.0.2 

@cody-greene
Copy link

Why does node-sass even include a --watch option? It just seems out of scope. In fact, other tools [including but not limited to] webpack need to set up their own file watchers to properly trigger auto reloading and hot swapping. Including this functionality in node-sass increases the surface area for bugs, , expands the required API and documentation, and inflates the dependency tree. And now the dependencies that gaze is using are printing multiple warnings because they're so old.

Why not improve the focus of this package and remove the --watch option entirely?

@callumlocke
Copy link

@cody-greene I think there is some value in having it, but agree it should be separated out from this module, eg sass vs sass-cli. Lots of other modules have made this change recently and it works well imo

@cody-greene
Copy link

@callumlocke Yes there is value in the feature. I can understand a few people are definitely using it. But I meant to suggest that the value provided is vastly overshadowed by these other issues.

@GongT
Copy link

GongT commented Jan 13, 2017

Can I install a "chokidar version" of node-sass now 😍?!

node-sass can't watch files for a long time, and do not know why, now I'm using nodemon around node-sass... LOL

@michaelwayman
Copy link

michaelwayman commented Apr 15, 2017

Can I open a PR to switch to Chokidar?

The use of Gaze makes my CPU burn at 150-300% when using with docker for mac, with Chokidar my CPU runs at a normal 6-10%.

I will start working on this as it really prevents me from using node-sass, or docker-for-mac.

What do you need to see in my PR to get it merged?

Edit: I see a PR already open to do this #844
What is the holdup?

xzyfer added a commit to xzyfer/node-sass that referenced this issue Jun 15, 2017
This logic is all tightly coupled in the bin. This logic has been
notoriously impossible to test due to weird fs timing issues. By
extracting the actual logic we're now able to test it in isolation.

With this in place replacing Gaze (sass#636) becomes a viable option.

Closes sass#1896
Fixes sass#1891
xzyfer added a commit to xzyfer/node-sass that referenced this issue Jun 15, 2017
This logic is all tightly coupled in the bin. This logic has been
notoriously impossible to test due to weird fs timing issues. By
extracting the actual logic we're now able to test it in isolation.

With this in place replacing Gaze (sass#636) becomes a viable option.
This PR not only massively increases our test coverage for the
watcher but also address a bunch of known edge cases i.e. orphaned
imports when a files is deleted.

Closes sass#1896
Fixes sass#1891
xzyfer added a commit to xzyfer/node-sass that referenced this issue Jun 15, 2017
This logic is all tightly coupled in the bin. This logic has been
notoriously impossible to test due to weird fs timing issues. By
extracting the actual logic we're now able to test it in isolation.

With this in place replacing Gaze (sass#636) becomes a viable option.
This PR not only massively increases our test coverage for the
watcher but also address a bunch of known edge cases i.e. orphaned
imports when a files is deleted.

Closes sass#1896
Fixes sass#1891
xzyfer added a commit to xzyfer/node-sass that referenced this issue Jun 15, 2017
This logic is all tightly coupled in the bin. This logic has been
notoriously impossible to test due to weird fs timing issues. By
extracting the actual logic we're now able to test it in isolation.

With this in place replacing Gaze (sass#636) becomes a viable option.
This PR not only massively increases our test coverage for the
watcher but also address a bunch of known edge cases i.e. orphaned
imports when a files is deleted.

Closes sass#1896
Fixes sass#1891
xzyfer added a commit to xzyfer/node-sass that referenced this issue Jun 15, 2017
This logic is all tightly coupled in the bin. This logic has been
notoriously impossible to test due to weird fs timing issues. By
extracting the actual logic we're now able to test it in isolation.

With this in place replacing Gaze (sass#636) becomes a viable option.
This PR not only massively increases our test coverage for the
watcher but also address a bunch of known edge cases i.e. orphaned
imports when a files is deleted.

Closes sass#1896
Fixes sass#1891
xzyfer added a commit that referenced this issue Jun 17, 2017
This logic is all tightly coupled in the bin. This logic has been
notoriously impossible to test due to weird fs timing issues. By
extracting the actual logic we're now able to test it in isolation.

With this in place replacing Gaze (#636) becomes a viable option.
This PR not only massively increases our test coverage for the
watcher but also address a bunch of known edge cases i.e. orphaned
imports when a files is deleted.

Closes #1896
Fixes #1891
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.