OfficeDev / office-ui-fabric-core

The front-end CSS framework for building experiences for Office and Microsoft 365.
https://developer.microsoft.com/en-us/fabric
Other
3.77k stars 465 forks source link

Fabric sass not compile #1192

Closed chendjouCaleb closed 4 years ago

chendjouCaleb commented 5 years ago

Why I can not compile the Fabric.scss file

Jahnp commented 5 years ago

Hi @chendjouCaleb , can you please share more details about your environment (OS, node/npm version, etc), and what specific errors you're getting?

chendjouCaleb commented 5 years ago

I use windows 10, npm version 6.9.0.

Here is a trace of the error when I try to compile the Fabric.scss file.

Error: $string: ms-fadeIn is not a string. ╷ 181 │ $namelist: append($namelist, unquote($newname), 'comma'); │ ^^^^^^^^^^^^^^^^^ ╵ node_modules\office-ui-fabric-core\dist\sass\mixins\_animation.mixins.scss 181:34 ms-animation() node_modules\office-ui-fabric-core\dist\sass\mixins\_animation.mixins.scss 209:5 @content node_modules\office-ui-fabric-core\dist\sass\mixins\_directionality.mixins.scss 11:5 ms-LTR() node_modules\office-ui-fabric-core\dist\sass\mixins\_animation.mixins.scss 208:3 ms-slideRightIn10() node_modules\office-ui-fabric-core\dist\sass\_animation.scss 13:3 @import node_modules\office-ui-fabric-core\dist\sass\fabric.scss 12:9 root stylesheet

chendjouCaleb commented 5 years ago

Please I wait a response

viceice commented 4 years ago

Same here :-( image

chendjouCaleb commented 4 years ago

The file (_Animation.Mixins.MDL2scss) in which this error occurs is deprecated. You can solve the problem by remove the import of this file on the file _Animation.Mixins.scss.

viceice commented 4 years ago

but on every reinstall it would be reset to default. So this should be fixed. I can send a pr to fix this.

malsabbagh commented 4 years ago

@ViceIce did you open a PR for this?

viceice commented 4 years ago

Not yet. Can do it the next days.

chendjouCaleb commented 4 years ago

I updated node-sass, style-loader, sass-loader and it works

josteink commented 4 years ago

This error is encountered when used together with both Angular 8.x and Angular 9.

ERROR in ./src/styles.scss (./node_modules/@angular-devkit/build-angular/src/angular-cli-files/plugins/raw-css-loader.js!./node_modules/postcss-loader/src??embedded!./node_modules/sass-loader/lib/loader.js??ref--15-3!./src/styles.scss)
Module build failed (from ./node_modules/sass-loader/lib/loader.js):

    $namelist: append($namelist, unquote($newname), 'comma');
                                ^
      $string: ms-fadeIn  is not a string.
    ╷
177 │     $namelist: append($namelist, unquote($newname), 'comma');
    │                                  ^^^^^^^^^^^^^^^^^
    ╵
  ../../../node_modules/office-ui-fabric-core/dist/sass/mixins/_Animation.Mixins.MDL2.scss 177:34  ms-animation()
  ../../../node_modules/office-ui-fabric-core/dist/sass/mixins/_Animation.Mixins.MDL2.scss 205:5   @content
  ../../../node_modules/office-ui-fabric-core/dist/sass/mixins/_Directionality.Mixins.scss 11:5    ms-LTR()
  ../../../node_modules/office-ui-fabric-core/dist/sass/mixins/_Animation.Mixins.MDL2.scss 204:3   ms-slideRightIn10()
  ../../../node_modules/office-ui-fabric-core/dist/sass/_Animation.MDL2.scss 10:3                  @import
  ../../../node_modules/office-ui-fabric-core/dist/sass/_Animation.scss 58:9                       @import
  ../../../node_modules/office-ui-fabric-core/dist/sass/Fabric.scss 12:9                           @import
  stdin 8:9                                                                                        root stylesheet
      in /home/jostein/build/OutlookAddIn/OutlookAddIn/ClientApp/node_modules/office-ui-fabric-core/dist/sass/mixins/_Animation.Mixins.MDL2.scss (line 177, column 34)
Error: 
    $namelist: append($namelist, unquote($newname), 'comma');
                                ^
      $string: ms-fadeIn  is not a string.

With submitted PR, this compilation-error goes away.

chendjouCaleb commented 4 years ago

Add the latest version of node-sass to the project

josteink commented 4 years ago

Add the latest version of node-sass to the project

I'm using Angular 8.x and there is no direct (or indirect) dependency on node-sass, only on sass and sass-loader.

I'm able to upgrade sass via tweaking package-lock.json, but that doesn't fix the error.

Updating sass-loader (which is the component reporting the error here) doesn't seem to be possible, as package-lock.json keeps reverting to an older version.

josteink commented 4 years ago

Hopefully this PR will result in a newer version being made available which fixes this without any need for dirty hacks.

chendjouCaleb commented 4 years ago

Add the latest version of node-sass to the project. npm install node-sass

Will solve the problem.

josteink commented 4 years ago

Can confirm. That's amazing. Thanks for the feedback!

DyuldinKS commented 3 years ago

Updated the following packages, but still it doesn't work - have the same error with "@fluentui/react": "^7.149.4", and "office-ui-fabric-core": "^11.0.0"

success Saved 4 new dependencies.
info Direct dependencies
├─ node-sass@5.0.0
├─ sass-loader@10.0.5
├─ sass@1.29.0
└─ style-loader@2.0.0
sawanm9000 commented 3 years ago

I'm getting the same problem with the latest version of sass. I think this issue needs to be reopened, @lindalu-MSFT.

valmonten commented 3 years ago

There are security concerns around using node-sass. Can this be upgraded to dart sass? Also if node-sass is really a dependency of office-ui-fabric-core then why is it not listed as a dependency of the project in the dependencies tab here? https://www.npmjs.com/package/office-ui-fabric-react

KevinTCoughlin commented 3 years ago

The above linked PR should fix the root cause of this error seen on dart-sass and other modern implementations. You can workaround it by patching locally:

// Outputs the properties for an animation.
@mixin ms-animation($names, $duration, $timing: $ms-animation-ease-1, $fillMode: both) {
  // Set a default value for the version-scoped variable if it isn't assigned.
  $ms-fabric-version-suffix: '' !default;
  // Append the scoping suffix to each animation name.
  $namelist: ();
  @each $name in $names {
    $newname: $name#{$ms-fabric-version-suffix};
    $namelist: append($namelist, $newname, 'comma');
  }

  // Output the animation's properties.
  animation-name: $namelist;
  animation-duration: $duration;
  animation-timing-function: $timing;
  animation-fill-mode: $fillMode;
  // Make the animation instant for users who prefer no motion.
  @include ms-prefers-reduced-motion {
    animation: none;
  }
}
theofred commented 3 years ago

None of the solutions above worked for me.

@KevinTCoughlin how should I patch it locally? Do you mean by changing the code inside my dependencies, or where should I put your mixin to properly overwrite the other one?