feat: improve sass compilation #109
Conversation
src/loaders/sass.ts
Outdated
| const compileString = await import("sass").then(r => r.compileString || r.default.compileString); | ||
|
|
||
| const output: LoaderResult = []; | ||
|
|
||
| const contents = await input.getContents(); | ||
|
|
||
| output.push({ | ||
| contents: compileString(contents).css, | ||
| contents: compileString(contents, { loadPaths: [dirname(input.srcPath)] }).css, |
There was a problem hiding this comment.
loadPaths is required to enable relative imports (sass/sass#3366 (comment))
There was a problem hiding this comment.
@dwightjack what about adding 'node_modules' ? Just in case we want to add @use '@external-node-module/style.css'
There was a problem hiding this comment.
@cpreston321 thanks for your feedback. I update the PR including node_modules and added a test (0141120).
There was a problem hiding this comment.
Hey @dwightjack I appreciate it! ✅ It would be nice to see in the future to see options that we can pass the sass complier like compressed: true etc.
Codecov Report
@@ Coverage Diff @@
## main #109 +/- ##
==========================================
+ Coverage 95.95% 96.09% +0.14%
==========================================
Files 8 8
Lines 470 487 +17
Branches 96 98 +2
==========================================
+ Hits 451 468 +17
Misses 19 19
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@manniL Think we can get some eyes on this ? 🙏🏼 |
|
@cpreston321 (cc @manniL) thanks! While I confirmed that the latest changes to load from About this comment, i guess that implementing a feature such as configurable loaders would be helpful, but it is, in my opinion, a pretty important change so I'd like to have feedback from the maintainers about this. Another note: since the PR might need some more reviews and implementations, what about moving this change to another small PR so that we can close nuxt/module-builder#67 once published? |
|
dwightjack
force-pushed
the
feat/sass-improvements
branch
from
0141120 to
cb0ee73
Compare
|
@cpreston321 sorry for the late reply. I extracted the file extension fix to another PR (#111) and rebased this PR onto the main branch without that change. |
|
Do you mind to rebase PR? |
|
Any progress with this? I'm happy to help if there are open tasks. |
|
@pi0 sorry, had a rough few days and hadn't the chance to work on this. Thanks for merging the PR. Let me know if there's anything else I could do to improve this feature. Of course I am available to work on other issues as well 😃 |
|
No problems. I hope you have better days ahead 💚 And for sure looking forward for your next amazing works 🚀 |
|
Thanks guys :D |
This PR should fix nuxt/module-builder#67 and add some improvements to the sass loader.
Changes:
options.ext. I don't see any use case where we'd want an extensions different from.csssince sass loader compiles the source to CSS.@useand@importrules in sass and Vue SFC files._because they are sass partials.