thgh / rollup-plugin-scss

Rollup and compile multiple .scss, .sass and .css imports
MIT License
135 stars 46 forks source link

Adds fix to add include path before processing when output is false #46

Closed mach25 closed 4 years ago

mach25 commented 4 years ago

I had a problem with relative paths when trying to add scss files as modules when they contain @import with relative paths.

Noticed that includePaths.push(dirname(id)) was done after processing and returning the result and the include path for the file was not added to node-sass before processing the file.

So i moved the line up a bit and added a comment which seemed to work ok

nhoizey commented 4 years ago

I'm using rollup-plugin-scss in https://github.com/nhoizey/pack11ty/blob/master/rollup.config.js and I indeed noticed I had to @import Sass files with a path relative to Rollup instead of the parent Sass file: https://github.com/nhoizey/pack11ty/blob/master/src/_assets/sass/additional.scss#L2-L5

However, it was not necessary in another parent Sass file build in another of my Rollup builds: https://github.com/nhoizey/pack11ty/blob/master/src/_assets/sass/critical.scss#L1-L19

It looks odd to have this only once.

mach25 commented 4 years ago

@nhoizey it's the same deal when using the watch option and this patch will fix that too. The watches were added before adding the include path

thgh commented 4 years ago

It looks like the includePaths approach of this plugin might result in race conditions.

I will just merge it and hope it works out!

thgh commented 4 years ago

Release in v2.2.0