sass loader prefers node-sass if it exists anywhere in the dep tree

[robjtede]

🐞 Bug report

Command (mark with an x)

• build
• serve

Is this a regression?

Probably not.

Description

Angular CLI prefers node-sass if it exists anywhere in the dependency tree.

This has manifested during an update of ngx-markdown from 10.0.0 to 10.1.0 in which one of it's dependency has added node-sass as a dependency. The error thrown relates to the SASS implementation not understanding SASS modules.

The offending line in Angular CLI goes against a change that was made in sass-loader v9 to prefer dart-sass. Since the current CLI next version of build-angular (~0.1001.0-next.2) updates to sass-loader v9, I feel it would be appropriate to either match its behavior or delegate the choice to sass-loader (by not explicitly providing the implementation option. If that is considered a breaking change, then perhaps an explicit developer choice is made available under stylePreprocessorOptions` or something similar.

🔬 Minimal Reproduction

Steps and notes in this repo: https://github.com/robjtede/repro-sass-loading

🔥 Exception or Error

ERROR in ./src/styles.scss (./node_modules/css-loader/dist/cjs.js??ref--14-1!./node_modules/postcss-loader/src??embedded!./node_modules/resolve-url-loader??ref--14-3!./node_modules/sass-loader/dist/cjs.js??ref--14-4!./src/styles.scss)
Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: Invalid CSS after "...@include sample": expected "}", was ".yellowBg;"
        on line 4 of src/styles.scss
>>     @include sample.yellowBg;

   -------------^

🌍 Your Environment

Angular CLI: 10.1.0-next.2
Node: 14.6.0
OS: darwin x64

Angular: 10.0.5
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.1001.0-next.2
@angular-devkit/build-angular     0.1001.0-next.2
@angular-devkit/build-optimizer   0.1001.0-next.2
@angular-devkit/build-webpack     0.1001.0-next.2
@angular-devkit/core              10.1.0-next.2
@angular-devkit/schematics        10.1.0-next.2
@angular/cli                      10.1.0-next.2
@ngtools/webpack                  10.1.0-next.2
@schematics/angular               10.1.0-next.2
@schematics/update                0.1001.0-next.2
rxjs                              6.5.5
typescript                        3.9.7
webpack                           4.43.0

Anything else relevant?

Related issues:

• 10.1.0 breaks my build - scss loader issue with sass import jfcere/ngx-markdown#252
• SassError: expected "{". webpack-contrib/sass-loader#867
• addition of node-sass is breaking angular builds joypixels/emoji-toolkit#32

[alan-agius4]

Hi @robjtede, the problem here is that emoji-toolkit is specifying devDepedencies as dependencies, thus I suggest to file an issue in their issue tracker.

Changing to logic to prefer sass instead of node-sass, will result in a breaking change with the risk of breaking users which depend on node-sass specific behaviour, also in a CLI project, sass is always available transitively via build-angular package.

In future versions of the CLI will be deprecating node-sass usage.

[robjtede]

Thanks for checking it out. Might be one to know about since I can't imagine ngx-markdown is the only package to unknowingly introduce breaking changes in a minor release. Waiting for a reply to the issue on emoji-toolkit.

An observation: it seems very difficult (if not impossible) to provide a strong semver guarantee in the precense of this conditional transitive dependency inclusion. I maintain for now that an explicit developer option at the Angular CLI level would have significant value given that we don't control every depth of dependencies we use.

[alan-agius4]

Hi @robjtede,

An observation: it seems very difficult (if not impossible) to provide a strong semver guarantee in the precense of this conditional transitive dependency inclusion. I maintain for now that an explicit developer option at the Angular CLI level would have significant value given that we don't control every depth of dependencies we use.

Unless, you include 3rd party build tools which set explicitly node-sass as a direct dependency, you shouldn't encounter this problem. And in such case that tool should likely add node-sass/sass dependencies as peerDependencies. IMHO these issues should be fixed upstream rather than providing an option to workaround it. We should consider adding this option in the future if we see this issue is being encountered more often, but this logic has been present in the CLI since version 8 and I don't recall anyone reported it before or a genuine case where adding an option to choose the SASS preprocessor was truly needed.

Adding an option to the Angular CLI increases the public API and increases maintenance especially considering we'd like to deprecate node-sass support altogether.

In addition to that, using a package lock you will guard yourself from accidental direct and transitive dependencies breakages.

[alan-agius4]

Closing as this is not actionable by us. The fix should be done upstream in the emoji-toolkit project.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}