Skip to content

replace node-sass to dart-sass #56

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

Merged
merged 2 commits into from
Apr 11, 2019
Merged

replace node-sass to dart-sass #56

merged 2 commits into from
Apr 11, 2019

Conversation

pikax
Copy link
Member

@pikax pikax commented Mar 20, 2019

Dart Sass is the primary implementation of Sass

Replacing node-sass to dart-sass since is the primary implementation and doesn't require to be compiled.

#50

@@ -57,7 +57,7 @@ test('preprocess sass', () => {
const style = parse({
source:
'<style lang="sass">\n' +
'$red: rgb(255, 0, 0);\n' +
'$red: rgb(255, 0, 0)\n' +
Copy link
Member

Choose a reason for hiding this comment

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

It would be a breaking change IMO. Though dart-sass & node-sass are both compliant with the sass spec, there're many subtle differences underlyingly, especially when it comes to error tolerance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it worth the breaking change for not be needing to compile node-sass?

Copy link
Member

Choose a reason for hiding this comment

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

It won't affect vue-loader but rollup-plugin-vue would need a major bump /cc @znck

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it would be a major bump for compiler utils as well as RPV.

Copy link
Member

Choose a reason for hiding this comment

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

So, let's leave it up to you to decide when to merge this PR and release a new major for both libraries 👀

@znck znck merged commit 3e4e3fc into vuejs:master Apr 11, 2019
tim-janik added a commit to tim-janik/beast that referenced this pull request Jun 29, 2020
…r-utils

Patch @vue/component-compiler-utils so that the Sass module can be
configured similar to how rollup-plugin-scss allows it.
In vuejs/component-compiler-utils#56, the use
of 'sass' was hardcoded, which prevents use of node-sass features in
Vue SFCs, like node-sass functions (needed by chromatic-sass).

Beyond that, due to Sass and CSS providing clashing function names like
min()/max(), the Dart 'sass' module chokes on the simplest of expressions:
            * { width: min(2, 3) * 1px; }

Signed-off-by: Tim Janik <timj@gnu.org>
tim-janik added a commit to tim-janik/beast that referenced this pull request Jun 29, 2020
* chromatic-sass:
  EBEAST: cssaux.scss: lcolor(): adjust color to specific perceptible lightness
  EBEAST: cssaux.scss: add lgrey() to create shade from perceptible lightness
  EBEAST: cssaux.scss: add ipow()
  EBEAST: cssaux.scss: add clamp(), contrast-lighten() and contrast-darken()
  EBEAST: cssaux.scss: add asfactor(), darker(), lighter()
  EBEAST: theme.scss: rename (from variables.scss)
  EBEAST: rollup.config.js: use 'node-sass' for Vue SFC
  EBEAST: chromatic-sass: add and utilize chromatic-sass in scss sources
  EBEAST: fix-vue-ccu.diff: add 'sass' option to @vue/component-compiler-utils
	Patch @vue/component-compiler-utils so that the Sass module can be
	configured similar to how rollup-plugin-scss allows it.
	In vuejs/component-compiler-utils#56, the use
	of 'sass' was hardcoded, which prevents use of node-sass features in
	Vue SFCs, like node-sass functions (needed by chromatic-sass).
	Beyond that, due to Sass and CSS providing clashing function names like
	min()/max(), the Dart 'sass' module chokes on the simplest of expressions:
	            * { width: min(2, 3) * 1px; }
  EBEAST: package.json.in: upgrade rollup-plugin-vue

Signed-off-by: Tim Janik <timj@gnu.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants