-
-
Notifications
You must be signed in to change notification settings - Fork 428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: sass modules "@use" #770
test: sass modules "@use" #770
Conversation
Codecov Report
@@ Coverage Diff @@
## master #770 +/- ##
=======================================
Coverage 97.75% 97.75%
=======================================
Files 10 10
Lines 178 178
Branches 63 63
=======================================
Hits 174 174
Misses 4 4 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job! You are wonderful!
Big thanks! |
Sorry, does it mean I can use |
@egze only thing you need is to install |
When using dart sass, |
@mitinarseny I tested it and it does resolve it - things like |
@donnysim Sure:
# make sure that node_modules/katex/dist/katex.css exists
$ ls node_modules/katex/dist
README.md contrib fonts katex.css katex.js katex.min.css katex.min.js katex.mjs // in main.js
import './sass/main.scss'; $ tree sass
sass
├── abstracts
│ ├── _index.scss
│ └── _variables.scss
├── base
│ ├── _index.scss
│ ├── _reset.scss
│ └── _typography.scss
├── layout
│ ├── _grid.scss
│ └── _index.scss
└── main.scss // sass/main.scss
@charset 'utf-8';
@use 'abstracts';
@use 'base';
@use 'layout'; // sass/base/_index.scss
@import 'reset';
@import 'typography'; // sass/base/_typography.scss
@use '~katex/dist/katex.css';
@use '~firacode/distr/fira_code.css'; // in webpack.config.js
{
test: /\.s[ac]ss$/i,
use: [
'style-loader',
'css-loader',
{
loader: 'postcss-loader', // Run postcss actions
options: {
plugins: function () { // postcss plugins, can be exported to postcss.config.js
return [
require('autoprefixer')
];
}
}
},
{
loader: 'sass-loader',
options: {
// Prefer `dart-sass`
implementation: require('sass'),
sassOptions: {
fiber: require('fibers'),
},
},
},
],
} |
@mitinarseny you could try removing the .css extension - |
@mitinarseny @evilebottnawi is that check still necessary? I don't see a way to get current implementation to skip it on |
Nope. It does not have any influence, it's just a recommendation according to sass docs. Anyway, this is solution which I came to, too:
And it works, but you should remove import 'katex/dist/katex'; BTW, |
I'm currently investigating to start using "@use", and I wonder if it works if you have multiple component-a.js
component-b.js
And because of this, the styling inside the "base" sass file will be included only once, and in the correct order (this is how I have been doing sass modules for quite some while, and it works so great for only having the things used in your bundle). If I would add this as an import in the individual sass files for the components, someone that imports both components will end up with duplicate styling rules in the result. I saw '@use', and I wonder if this could remove this extra import in favor of having it in the scss file instead, which makes more sense? Because the sass-loader triggers for each separate |
This PR contains a:
Motivation / Use-Case
Sass launched modules feature not too long ago and its path resolution works the same way as
@import
does, no changes are required to support it. There are slight differences for@use
vs@import
when testing:@use
are resolved only once no matter how many times the same file is included;~bootstrap
require alias -@use "~bootstrap" as bootstrap
because~bootstrap
is invalid name in sass;@use url
- that's@import
responsibility;@use
in one line separated by comma - each@use
must be on new line;.class { @import "path" }
;At the moment only Dart Sass 1.23.0 and up support
@use
feature so bumped sass dependency.Breaking Changes
None.
Additional Info
http://sass.logdown.com/posts/7858341-the-module-system-is-launched