Replace SCSS Lint with Stylelint #5448
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@decabeza Hope you like this one! 😄 (If you don't, I'm afraid there isn't much we can do about it, though 😅). |
javierm
force-pushed
the
stylelint
branch
2 times, most recently
from
dfca3e0
to
6041ab6
Compare
javierm
force-pushed
the
stylelint
branch
3 times, most recently
from
017b057
to
1eba568
Compare
SCSS Lint was based on Ruby Sass, which has been deprecated since 2018
and doesn't support some of the latest features in Dart Sass.
Since we're going to migrate to Dart Sass, we're also migrating to
Stylelint. In order to provide all the funcionality SCSS Lint had, we
also need to install a few Stylelint plugins. We also need to install
the `postcss-scss` package so Stylelint can read SCSS files.
We're still getting the linting errors we used to get in
`legislation_process.scss` because of the `max-nesting-depth` and
`selector-max-compound-selectors` rules, as well as the warnings we used
to get in `layout.scss` because the `ssb-whatsapp_app` HTML class
contains undescores in their name.
We're also getting a couple of new linting errors, which could be false
positives:
* The `scss/no-unused-private-members` rule reports errors in
`_consul_settings.scss` because of the `$-zf-*` variables, but I'm not
too worried about this one because these lines won't be necessary after
updating Foundation.
* The `scss/selector-no-redundant-nesting-selector` reports an error in
`datepicker_overrides.scss`, but removing the unnecessary nesting
selector would make it harder to understand the code (assuming it'd
work in the first place).
We've changed some files due to few differences between SCSS Lint rules
and their Stylelint equivalents:
* The `@stylistic/string-quotes` rule detects a case in `admin.scss`
that StringQuotes didn't detect.
* The `function-url-quotes` rule detects a case in `mixins/icons.scss`
that UrlQuotes didn't detect.
* The `@stylistic/declaration-bang-space-before` rule detects a case in
`sdg/goals/show.scss` that BangFormat didn't detect.
There are also a couple of rules that don't behave exactly like they
used to:
* The equivalents of SpaceBetweenParens and SpaceAfterComma don't cover
parenthesis or commas in mixin parameters; we haven't found rules that
detect these cases.
* DisableLinterReason probably has an equivalent that behaves
differently but, since we never disable linters inline, we aren't
adding its equivalent rule.
Note we're removing the SpaceAfterVariableColon rule because its
equivalent, `scss/dollar-variable-colon-space-after`, reports cases
where we add spaces to indent several variable assignments (which we do
a lot in the `_consul_settings.scss` file). We might add this rule again
if we stop aligning consecutive assignments.
We're also removing the QualifyingElement rule because its equivalent,
`selector-no-qualifying-type: true`, behaves differently. For example,
in this code:
```
a.qualifying {
color: inherit;
}
p {
&.qualifying {
color: inherit;
}
}
```
With the QualifyingElement from SCSS Lint, the first rule is incorrect
but the second one is correct. With the selector-no-qualifying-type rule
from Stylelint, on the other hand, both rules are incorrect.
Personally, I never liked the QualifyingElement rule, and we were
working around it anyway, so we aren't applying
selector-no-qualifying-type.
For reference, here's a full list of the SCSS Lint rules we had enabled
and their Stylelint equivalents.
BangFormat
"@stylistic/declaration-bang-space-after": "never"
"@stylistic/declaration-bang-space-before": "always"
BorderZero
declaration-property-value-disallowed-list:
border:
- none
ColorKeyword
color-named: "never"
DebugStatement
at-rule-disallowed-list:
- debug
DeclarationOrder
order/order:
- dollar-variables
- custom-properties
- type: at-rule
name: extend
- type: at-rule
name: include
hasBlock: false
- declarations
- type: at-rule
name: include
hasBlock: true
- rules
ElsePlacement # Apparently replaced by the combination of:
scss/at-else-closing-brace-space-after: always-intermediate
scss/at-else-empty-line-before: never
scss/at-if-closing-brace-space-after: always-intermediate
scss/at-if-closing-brace-newline-after: always-last-in-chain
EmptyLineBetweenBlocks:
ignore_single_line_blocks: true
rule-empty-line-before:
- "always-multi-line"
- ignore:
- after-comment
- first-nested
EmptyRule:
block-no-empty: true
FinalNewline
"@stylistic/no-missing-end-of-source-newline": true
HexLength
color-hex-length: "short"
HexNotation
@stylistic/color-hex-case: "lower"
HexValidation
color-no-invalid-hex: true
IdSelector
selector-max-id: 0
ImportPath
scss/load-no-partial-leading-underscore: true
scss/at-import-partial-extension: never
Indentation
"@stylistic/indentation": 2
LeadingZero
@stylistic/number-leading-zero: "always"
NameFormat
scss/at-function-pattern: "^(-?[a-z][a-z0-9]*)(-[a-z0-9]+)*$"
scss/at-mixin-pattern: "^(-?[a-z][a-z0-9]*)(-[a-z0-9]+)*$"
scss/dollar-variable-pattern: "^(-?[a-z][a-z0-9]*)(-[a-z0-9]+)*$"
scss/percent-placeholder-pattern: "^(-?[a-z][a-z0-9]*)(-[a-z0-9]+)*$"
NestingDepth
max-nesting-depth: 4
PlaceholderInExtend
scss/at-extend-no-missing-placeholder: true
PrivateNamingConvention
scss/no-unused-private-members: true
PropertySpelling
property-no-unknown: true
PseudoElement
selector-pseudo-element-colon-notation: "double"
selector-pseudo-element-no-unknown: true
SelectorDepth
selector-max-compound-selectors: 5
SelectorFormat # Not always followed; ssb-whatsapp_app
custom-property-pattern: "^([a-z][a-z0-9]*)(-[a-z0-9]+)*$"
selector-class-pattern: "^([a-z][a-z0-9]*)(-[a-z0-9]+)*$"
Shorthand
shorthand-property-no-redundant-values: true
SingleLinePerProperty
"@stylistic/declaration-block-semicolon-newline-after": always-multi-line
SingleLinePerSelector
@stylistic/selector-list-comma-newline-after": always
SpaceAfterComma
"@stylistic/value-list-comma-space-after": always
SpaceAfterPropertyColon
"@stylistic/declaration-colon-space-after": always-single-line
SpaceAfterPropertyName
"@stylistic/declaration-colon-space-before": never
SpaceAfterVariableName
scss/dollar-variable-colon-space-before: never
SpaceAroundOperator
scss/operator-no-unspaced: true
SpaceBeforeBrace
@stylistic/block-opening-brace-space-before: always
SpaceBetweenParens
"@stylistic/function-parentheses-space-inside": never
"@stylistic/selector-attribute-brackets-space-inside": never
"@stylistic/selector-pseudo-class-parentheses-space-inside": never
"@stylistic/media-feature-parentheses-space-inside": never
StringQuotes
"@stylistic/string-quotes": double
TrailingSemicolon
"@stylistic/declaration-block-trailing-semicolon": always
TrailingWhitespace # Note it was enabled by the gem and not by us
"@stylistic/no-eol-whitespace": true
TrailingZero
"@stylistic/number-no-trailing-zeros": true
UnnecessaryMantissa
"@stylistic/number-no-trailing-zeros": true
UnnecessaryParentReference
scss/selector-no-redundant-nesting-selector: true
UrlQuotes
function-url-quotes: always
VendorPrefixes
value-no-vendor-prefix: true
selector-no-vendor-prefix: true
property-no-vendor-prefix: true
at-rule-no-vendor-prefix: true
media-feature-name-no-vendor-prefix: true
ZeroUnit:
length-zero-no-unit: true
This way we've been able to detect a bug where we were assigning a value to the `$global-left` variable instead of assigning it to either the `left` or `right` properties. This, however, was the right behavior, since adding a `left` indentation to the link wasn't correct because its parent element already had padding on the sides.
javierm
changed the base branch from
master
to
bump_jquery_file_upload_to_9.34.0
taitus
approved these changes
Apr 3, 2024
This was referenced Apr 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
Notes
Check the commit message for an explanation regarding which Stylelint rules are equivalent to the SCSS Lint rules we were using.
Most of the syntax problems reported by Stylelint are the same that SCSS Lint used to report; see issue #1676.