-
Notifications
You must be signed in to change notification settings - Fork 215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to dart-sass from deprecated node-sass #327
Conversation
@bsteinbk @GerardasB I'm running into an error in ui-test-app related to this change. Would you mind taking a look?
|
-o-transition: color background-color.2s ease; | ||
-webkit-transition: color background-color .2s ease; | ||
-moz-transition: color background-color .2s ease; | ||
-o-transition: color background-color .2s ease; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bsteinbk @GerardasB FWIW, I don't think you should ever need to use these vendor prefixes - I believe create-react-app will add these for you as a post-processing step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this was the responsibility of scss and one of the main reasons to use it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wgoehrig thanks for letting me know. I should've removed these rules instead of fixing immediate build issue.
Most of these rules were written before we moved to CRA, we will make sure not to add prefixes explicitly anymore and leave that for the tooling.
With node-sass now deprecated in favor of dart-sass, updating both the Extension webpack tools and all of our test apps to use it by default.
Per the sass-loader documentation, both are still supported and the node-sass vs. dart-sass is driven based on what is installed as a peer dependency. However the default will remain node-sass until the next major version of the sass-loader package.
fast-sass-loader does not intend to support dart-sass vs. node-sass so I'm removing support completely from our repo. The support for it will remain in our react-scripts fork until we move up to 4.0. Then at that time, depending on performance testing, we can decide to drop it or use the fork someone introduced (see yibn2008/fast-sass-loader#63 (comment))
Some additional details on the deprecation are here, https://sass-lang.com/blog/libsass-is-deprecated