Skip to content

Chokidar Updates - #4205

Merged
gaearon merged 4 commits into
react:nextfrom
originell:patch-1
May 20, 2018
Merged

Chokidar Updates#4205
gaearon merged 4 commits into
react:nextfrom
originell:patch-1

Conversation

@originell

@originell originell commented Mar 23, 2018

Copy link
Copy Markdown
Contributor

There have been a couple of changes to node-sass-chokidar over the recent days.

The following things have been changed, according to the node-sass-chokidar/CHANGELOG:

  • --recursive flag removed as it doesn't do anything (CHANGELOG v1.0.0)
  • initial scss build removed on watch-css, as chokidar changed the behaviour (CHANGELOG v1.0.0)

🍻

@facebook-github-bot

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@Timer

Timer commented Mar 26, 2018

Copy link
Copy Markdown
Contributor

/cc @michaelwayman I would love your LGTM

@petetnt petetnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a tiny nit 👍

@petetnt petetnt Mar 26, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove another --recursive flag here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the --recursive flag was removed in 1.0.0 and recursion is always on because there is no way to be non-recursive with chokidar.

Other than that this LGTM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the --include-path ./src isn't necessary, let's say you have files ./src/index.scss and ./src/_mixins.scss and in index.scss you @import './_mixins' it will still import correctly.

You don't necessarily need the --include-path ./src

Thanks @originell for keeping the docs updated with node-sass-chokidar, it had slipped my mind.

@originell

Copy link
Copy Markdown
Contributor Author

Changes landed. Also found another build-css I missed earlier. Hope that it's all good now :)

@michaelwayman

michaelwayman commented Mar 27, 2018

Copy link
Copy Markdown
Contributor

@Timer LGTM

@petetnt petetnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iansu

iansu commented Apr 10, 2018

Copy link
Copy Markdown
Contributor

Do we want this going into master or next?

@michaelwayman

Copy link
Copy Markdown
Contributor

This should go ahead and get merged now. node-sass-chokidar. Thanks @originell for the PR, this LGTM.

@gaearon

gaearon commented May 20, 2018

Copy link
Copy Markdown
Contributor

@iansu We don't plan to make more changes to master aside from critical fixes.

@gaearon
gaearon changed the base branch from master to next May 20, 2018 22:27
originell added 4 commits May 20, 2018 23:29
According to the changelog it is not necessary anymore as it doesn't do anything.
Chokidar now does this on it's own.
@gaearon
gaearon merged commit 0dfc6f6 into react:next May 20, 2018
@Timer Timer added this to the 2.0.0 milestone May 21, 2018
zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Sep 30, 2018
* Removes Chokidar Recursive Flag

According to the changelog it is not necessary anymore as it doesn't do anything.

* Removes initial build on SCSS watch

Chokidar now does this on it's own.

* Removes sass watch recursive, default include-path

as proposed by @michaelwayman

* Removes another left-over build-css
@lock lock Bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants