jfcere / ngx-markdown

Angular markdown component/directive/pipe/service to parse static, dynamic or remote content to HTML with syntax highlight and more...
https://jfcere.github.io/ngx-markdown
MIT License
1.06k stars 182 forks source link

10.1.0 breaks my build - scss loader issue with sass import #252

Closed qortex closed 4 years ago

qortex commented 4 years ago

The following SCSS code no longer compiles when updated to 10.1.0 (no issue when ngx-markdown is at 10.0.0 and everything the same otherwise):

@use 'sass:color';

@mixin uit-card-hint-text-component-theme($theme) {
  $hint-color: map-get($foreground, 'secondary-lighter-text');
  // element... {
        color: color.scale($hint-color, $lightness: -15%);

I now get the error at compile:

ERROR in ./src/styles.scss (./node_modules/css-loader/dist/cjs.js??ref--13-1!./node_modules/postcss-loader/src??embedded!./node_modules/resolve-url-loader??ref--13-3!./node_modules/sass-loader/dist/cjs.js??ref--13-4!./src/styles.scss)
Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: Invalid CSS after "...   color: color": expected expression (e.g. 1px, bold), was ".scale($hint-color,"
        on line 14 of src/app/backbone/card/ui-text-primitives-card/uit-card-hint-text/uit-card-hint-text.component.scss-theme.scss
        from line 3 of src/app/backbone/card/ui-text-primitives-card/ui-text-primitives-card.scss-theme.scss
        from line 2 of src/app/backbone/card/card.scss-theme.scss
        from line 1 of src/app/backbone/backbone.scss-theme.scss
        from line 13 of src/assets/scss/themes.scss
        from line 1 of /home/mic/dev/qontrol/product/frontend/src/styles.scss
>>         color: color.scale($hint-color, $lightness: -15%);

I'm at lost as to why this would occur. Any idea?

Code seems valid to me according to this page.

jfcere commented 4 years ago

Ho @qortex,

That's odd since the only changes in 10.1.0 are dependency updates for both Emoji-Toolkit and Katex.

Are you able to reproduce the problem in either stackblitz or a repository I could have access to?

qortex commented 4 years ago

yeah weird, I guess that's something with a scss/sass loader dependency that got updated and has a bug? I'll try to put something together. I'll let a few days pass and will check back later, it might get fixed upstream in the dependencies if it affects other people too.

jfcere commented 4 years ago

Strange thing is that the demo use SCSS with theming mixins and seems ok.

qortex commented 4 years ago

Yeah, I guess it's specifically linked to the @use 'sass:color'; bit.

robjtede commented 4 years ago

This is affecting my project too and I understand this is an oddity and seem weird to be related to ngx-markdown specifically.

The key observation, from my investigation, is that it seems sass-loader is preferring node-sass now but that has been provoked by this package dependency update of emoji-toolkit through ngx-markdown. Note that the sass-loader version is unchanged.

Before v10 upgrade:

› npm ls sass-loader sass node-sass
[redacted]
├─┬ @angular-devkit/build-angular@0.901.7
│ ├── sass@1.26.3
│ └── sass-loader@8.0.2
└─┬ @storybook/angular@5.3.19
  └── sass-loader@8.0.2  [deduped]

After v10 upgrade:

› npm ls sass-loader sass node-sass
[redacted]
├─┬ @angular-devkit/build-angular@0.1000.4
│ ├── sass@1.26.5
│ └── sass-loader@8.0.2
├─┬ @storybook/angular@5.3.19
│ └── sass-loader@8.0.2  [deduped]
└─┬ ngx-markdown@10.1.0
  └─┬ emoji-toolkit@6.0.0
    └── node-sass@4.14.1

It's not clear to me why emoji-toolkit needs node-sass as a normal dependency but that's likely not the best thing to address. To confirm this, one can rm -rf the node-sass package out of node_modules and suddenly builds start working; of course, that's not any kind of solution.

A final check: using the latest angular@next (now 10.1.0-next.2) with the version setup below. The same error persists indicating that node-sass is still being used. Note the increased version of sass-loader under @angular-devkit/build-angular. (To be extra sure the most recent version is used, deleting the other sass-loader version from node_modules produces the same result. It is only fixed, again, by deleting node-sass from node_modules.)

› npm ls sass-loader sass node-sass
[redacted]
├─┬ UNMET PEER DEPENDENCY @angular-devkit/build-angular@0.1001.0-next.2
│ ├── sass@1.26.10
│ └── sass-loader@9.0.2
├─┬ @storybook/angular@5.3.19
│ └── sass-loader@8.0.2
└─┬ ngx-markdown@10.1.0
  └─┬ emoji-toolkit@6.0.0
    └── node-sass@4.14.1

Relevant links:

jfcere commented 4 years ago

Hi @robjtede,

Interesting finding, Emoji-Toolkit did add node-sass@4.14.1 as a dependency while it was not the case before (see https://github.com/joypixels/emoji-toolkit/commit/d291f3f0fe04d650fcf5127c5dd58a325ad61c0e#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R44).

Still, why is it bringing the compilation error is beyond my understanding... is this a problem with sass-loader or node-sass?

robjtede commented 4 years ago

It's not clear which package in the chain should make a change to remedy this. I think the most reasonable is for angular-devkit to provide the explcit override to sass-loader.

For now, I'm just going to use ngx-markdown@~10.0.0 to avoid all this.

jfcere commented 4 years ago

I followed your issue on sass-loader repo, I think a simple reproduction repository would be appropriate to validate the problem and provide insights as who should fix what...

Nevertheless, thanks for the follow-up and the pushing on this issue, it is really appreciated!

robjtede commented 4 years ago

Created a minimal repro repo: https://github.com/robjtede/repro-sass-loading

Edit: angular-cli is preferring node-sass linked to offending line in readme of repo.

robjtede commented 4 years ago

https://github.com/angular/angular-cli/issues/18389

Update: sass-loader says it's angular-cli's problem, and angular-cli says it's emoji-toolkit's problem.

jfcere commented 4 years ago

The answer on angular-cli repository make sense, node-sass should not be set as a dependency but as a devDependency... looking forward to Emoji-Toolkit's answer on the issue you reported.

Again, thanks and good job on the investigation/follow-up on this!

jfcere commented 4 years ago

The fix for Emoji-Toolkit has been released as 6.0.1.

I'll update the dependency tonight to be sure nobody end-up with the broken version.

jfcere commented 4 years ago

I bumped Emoji-Toolkit dependency to ^6.0.1, you can update ngx-markdown to v10.1.1