node-sass-chokidar icon indicating copy to clipboard operation
node-sass-chokidar copied to clipboard

Contribute to node-sass

Open xzyfer opened this issue 8 years ago • 6 comments

Hello 👋

I'm the maintainer of node-sass. I'd love for us to work together to move some of these changes upstream.

Forking node-sass in this way is going to quickly result in divergence, and may negatively impact the community. For instance we're actively working to overhaul the CLI. Notable we've recently made changes to the watcher integration to allow use to move away from Gaze. Previously this simple wasn't possible to due issue with testing the watcher functionality reliably.

We'd welcome an up-to-date PR for chokidar integration :)

xzyfer avatar Jun 18 '17 11:06 xzyfer

This is a marvelous idea. I noticed the issue with chokidar had been open for some time on node-sass and wasn't sure if it was a planned move, hence, node-sass-chokidar. I would love to make a PR to node-sass and put up a notice that node-sass-chokidar has become deprecated. I find myself a little busy this upcoming week but the change shouldn't take long and I will try to work on it asap. Thanks for reaching out and letting me know you are open to the change @xzyfer

michaelwayman avatar Jun 19 '17 01:06 michaelwayman

Awesome. That old PR has been problematic for a couple reasons

  • the potential for destabilising the watcher module was very high
  • until recently we weren't able to reliably test the watcher logic

We didn't believe the risk of potentially destabilizing the watcher for millions of users was worth the potential benefits. Now that we have confidence in our testing around the watcher the risk of destabilisation is much lower and we're very much open to making the switch if it benefits the wider community.

xzyfer avatar Jun 19 '17 01:06 xzyfer

Since node-sass compiles .css files in watch mode when using the same input and output folders (see 1, 2) and node-sass-chokidar does not, does this mean that this behavior will change once node-sass-chokidar is merged?

If not then this project should probably continue independently to provide a clean and reliable workaround, especially since it is recommended by Facebook for create-react-app as well.

Livven avatar Jun 25 '17 00:06 Livven

@xzyfer, @Livven is right. node-sass-chokidar is a great workaround for some currently unsolved problems in node-sass.

@michaelwayman please consider keeping node-sass-chokidar active at least until the following node-sass bug is resolved: https://github.com/sass/node-sass/issues/1758

RoyTinker avatar Dec 12 '17 23:12 RoyTinker

We're working on node-sass@5 which brings in many of the benefits of node-sass-chokidar.

https://github.com/sass/node-sass/pull/2312

xzyfer avatar Jun 08 '18 08:06 xzyfer

@xzyfer, @Livven is right. node-sass-chokidar is a great workaround for some currently unsolved problems in node-sass.

@michaelwayman please consider keeping node-sass-chokidar active at least until the following node-sass bug is resolved: sass/node-sass#1758

Definitely agree with @RoyTinker in keeping this project running separately. The smallest of changes within this wrapper really lends itself to usability on a day-to-day basis, and is in part due to being decoupled from the sass/node-sass project.

For example, something as simple and standard as compiling before watching has long been a part of node-sass-chokidar, despite being rejected several times on the main project. While this is on the radar for its next release, it's almost been exactly a year since @xzyfer's mention of node-sass@5: which is still yet to be released.

It's understandable that a vastly popular project like sass/node-sass needs time to ensure comprehensive checks are carried out before making any "drastic" changes. I think that's the exact reason why projects such as node-sass-chokidar should continue alongside to continue pushing forward. Thanks for your time and effort, @michaelwayman, really appreciate it! 👍

rdhar avatar Jul 04 '19 20:07 rdhar