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
Consistently quote Sass modules strings #11461
Conversation
212f339
to
38e427d
Compare
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.
Thank you for PR! Please check one comment
src/language-css/parser-postcss.js
Outdated
@@ -525,7 +525,7 @@ function parseNestedCSS(node, options) { | |||
return node; | |||
} | |||
|
|||
if (lowercasedName === "import") { | |||
if (["import", "use", "forward"].includes(lowercasedName)) { |
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.
Could you define a new function as following:
const moduleRuleNames = new Set(["import", "use", "forward"]);
function isModuleRuleName(name) {
return moduleRuleNames.has(name);
}
And replace this to
if (isModuleRuleName(lowercasedName)) {
?
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.
Done!
tests/format/scss/quotes/quotes.scss
Outdated
@@ -0,0 +1,2 @@ | |||
@use "module"; | |||
@forward "module"; |
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.
We need more tests here, because we use parser here and it can break output, please add:
@use 'library' with (
$black: #222,
$border-radius: 0.1rem
);
ideally even more
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.
Need more tests
@alexander-akait I’ve added more use cases in tests. |
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.
CI failed but no related to this PR. When we fixed CI, we'll merge this.
Fixes #11053.
Description
Checklist
docs/
directory).changelog_unreleased/*/XXXX.md
file followingchangelog_unreleased/TEMPLATE.md
.✨Try the playground for this PR✨