Skip to content

chore: update stylelint to v16 #84

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.

Sign up for GitHub

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

Merged
yokuze merged 1 commit into from
Jun 16, 2025
Merged

chore: update stylelint to v16 #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 76 additions & 35 deletions .stylelintrc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,54 +1,52 @@
# We are using stylelint-config-standard for all of the base rules designed more broadly
# for CSS. The stylelint-scss plugin provides us with more SCSS specific rules. Those
# rules are prefixed by `scss/`
extends: stylelint-config-standard
extends:
- "stylelint-config-standard"
plugins:
- stylelint-scss
- "stylelint-scss"
- "@stylistic/stylelint-plugin"
rules:
# Turn off the following rule since our ltr/rtl approach generally violates it.
no-descending-specificity: null
# Turn off config-standard rule in favor of the scss/at-rule-no-unknown rule
at-rule-no-unknown: null
scss/at-rule-no-unknown: true
# General rules
indentation: 3
linebreaks: unix
no-eol-whitespace: true
no-missing-end-of-source-newline: true
no-empty-first-line: true
max-line-length:
'@stylistic/indentation': 3
'@stylistic/linebreaks': unix
'@stylistic/no-eol-whitespace': true
'@stylistic/no-missing-end-of-source-newline': true
'@stylistic/no-empty-first-line': true
'@stylistic/max-line-length':
- 140
- ignorePattern:
- /https?://[0-9,a-z]*.*/
max-empty-lines: 1
string-quotes: single
'@stylistic/max-empty-lines': 1
'@stylistic/string-quotes': single
string-no-newline: true
color-hex-case: lower
'@stylistic/color-hex-case': lower
color-hex-length: long
color-no-invalid-hex: true
no-invalid-position-at-import-rule: null
number-leading-zero: always
number-no-trailing-zeros: true
unit-case: lower
'@stylistic/number-leading-zero': always
'@stylistic/number-no-trailing-zeros': true
'@stylistic/unit-case': lower
unit-no-unknown: true
property-no-unknown: true
block-no-empty: true
value-keyword-case: lower
length-zero-no-unit: true
function-comma-space-after: always-single-line
function-comma-space-before: never
function-parentheses-space-inside: never-single-line
'@stylistic/function-comma-space-after': always-single-line
'@stylistic/function-comma-space-before': never
'@stylistic/function-parentheses-space-inside': never-single-line
function-linear-gradient-no-nonstandard-direction: true
function-calc-no-invalid: true
function-calc-no-unspaced-operator: true
selector-pseudo-class-no-unknown: true
selector-pseudo-element-no-unknown: true
selector-type-no-unknown: true
declaration-block-no-duplicate-properties: true
declaration-block-no-shorthand-property-overrides: true
declaration-colon-space-after: always
declaration-colon-space-before: never
declaration-block-trailing-semicolon: always
'@stylistic/declaration-colon-space-after': always
'@stylistic/declaration-colon-space-before': never
'@stylistic/declaration-block-trailing-semicolon': always
declaration-empty-line-before: never
Copy link
Contributor

@kmuncie kmuncie Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know there is a desire for more flexibility with empty lines, and I've been thinking a lot about #82.

While I understand this perspective, I have some concerns about removing this constraint entirely and allowing arbitrary spacing based on personal preference. Without any constraints, we might see inconsistent spacing patterns like this example:

.btn-tertiary {

   background: gray;

   color: white;

   padding: 0.5rem 1rem;

   border: none;
   margin: 10px;

   &:hover {

      background: darkgray;

   }

   &:focus {

      outline: 2px solid lightgray;

   }

}

Regarding the other PR's concerns about our blank line principles: while I recognize that the current rules may feel restrictive, I don't believe they actually violate our stated principle. The principle states "Related statements that accomplish a single task should be grouped together without blank lines. Use one blank line to separate unrelated statements."

I'd argue that allowing arbitrary spacing between property declarations would violate this principle. While color and background-color may seem unrelated as individual properties, they're logically grouped because they all relate to the parent selector and together form the complete specification for that element. Many properties are closely related (such as display: flex; flex-direction: row;), and removing this rule would rely solely on convention and code review to prevent their separation.

As a potential middle ground that addresses both concerns, I'd suggest this rule set as a step toward more permissive formatting:

'@stylistic/declaration-empty-line-before': never
'@stylistic/rule-empty-line-before':
   - always
   - ignore:
      - after-comment
      - first-nested
      - blockless-after-blockless  # Helps with @else
   - ignoreAtRules:
      - else                       # Specifically ignore @else spacing
      - else-if                    # And @else if

This approach would enforce separation of nested blocks (better aligning with our blank line principles) while improving readability. If a set properties is excessively long they can easily be separated visually and with explanation by code comments. I believe these rules should be auto-fixable, allowing older codebases to be updated automatically, though this should be validated.

With these proposed rules, the formatting would look like this:

.btn-tertiary {
   // Colors
   background: gray;
   color: white;
   // Spacing
   padding: 0.5rem 1rem;
   margin: 10px;
   // Decoration
   border: none;

   &:hover {
      background: darkgray;
   }

   &:focus {
      outline: 2px solid lightgray;
   }

   @if $theme == 'dark' {
      border: 1px solid white;
   } @else {
      border: 1px solid black;
   }
}

Copy link
Contributor Author

@yokuze yokuze Jun 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was actually my intention with this PR to keep the result as close to our existing standards as possible, to avoid any linting/standards discussions for the upgrade PR if we could.

In the PR description above, I noted:

A few rules were not moved there and are now removed from this config:

  • declaration-empty-line-before: never
  • rule-empty-line-before: never

so unfortunately both of these rules are among the ones that were not re-implemented in the @stylelistic repo and the proposed config just results in "rule not found" errors.

I don't disagree with the need for some kind of constraints on blank lines, and at first glance I like the results of the proposed config! We'll just need to implement those two rules ourselves as a plugin in a subsequent PR.

Sorry, I should've called that out more clearly, especially since there was an open issue about the blank line standards 😁

Copy link
Contributor

@kmuncie kmuncie Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed separately, many stylistic rules were moved to stylistic... but the ones I commented on you are saying were not available are not in the deprecated list and thus not moved. They still exist in stylelint code and documentation as far as I can tell.

May need an audit of those rules you thought were moved/deprecated to be double sure. Thanks

Copy link
Contributor Author

@yokuze yokuze Jun 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Sorry about that—The vibes were good but reality was bad, if you know what I mean ;)

Me, the human, has re-checked the config file and rules and made a few changes:

  1. Restored the config for all rules that I thought were deprecated but weren't. (Only function-calc-no-invalid is removed without replacement.)
  2. For rules that moved to @stylistic, I added the @stylistic/ name prefix where the existing rule was rather than grouping @stylistic rules together so that it's easier to see what rules were removed and which were just renamed
  3. Restored the extends: stylelint-config-standard statement so that we continue to benefit from the default ruleset. This necessitated disabling some new-since-v14+ rules that were enabled by default and throwing errors. We can evaluate what those settings should be in another PR
  4. Moved SCSS-specific overrides to the new overrides section. Now, rules that need to be disabled for SCSS syntax support are only disabled for SCSS files.

I re-ran this updated config against our larger internal projects and verified that linting standards are enforced as before.

Copy link
Contributor

@kmuncie kmuncie Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the adjustments are much easier to understand now as a straight upgrade!

comment-whitespace-inside: always
rule-empty-line-before:
Expand All @@ -66,19 +64,62 @@ rules:
- first-nested
- inside-block
- blockless-after-same-name-blockless
block-opening-brace-space-before: always
block-closing-brace-newline-after:
'@stylistic/block-opening-brace-space-before': always
'@stylistic/block-closing-brace-newline-after':
- always
- ignoreAtRules:
- if
- else
at-rule-name-space-after: always
scss/at-else-closing-brace-newline-after: always-last-in-chain
scss/at-else-closing-brace-space-after: always-intermediate
scss/at-else-empty-line-before: never
scss/at-if-closing-brace-newline-after: always-last-in-chain
scss/at-if-closing-brace-space-after: always-intermediate
'@stylistic/at-rule-name-space-after': always
# End collection of rules for if/else statements
scss/at-extend-no-missing-placeholder: true
scss/at-function-parentheses-space-before: never
scss/at-mixin-parentheses-space-before: never
# Disable newly-added rules that cause linting errors until we can evaluate them
color-function-alias-notation: null
alpha-value-notation: null
shorthand-property-no-redundant-values: null
property-no-vendor-prefix: null
custom-property-pattern: null
selector-class-pattern: null
keyframes-name-pattern: null
selector-id-pattern: null
declaration-block-no-redundant-longhand-properties: null
media-feature-range-notation: null
selector-not-notation: null
color-function-notation: null
keyframe-selector-notation: null
at-rule-no-vendor-prefix: null
keyframe-block-no-duplicate-selectors: null
function-url-quotes: null
number-max-precision: null

# SCSS files require postcss-scss as customSyntax
# See: https://github.com/stylelint/stylelint/blob/a57d512dd5f79815a43e996a182b5e2a8154e363/docs/user-guide/configure.md#overrides
overrides:
-
files: [ "**/*.scss" ]
customSyntax: "postcss-scss"
rules:
# Turn off config-standard rule in favor of the scss/at-rule-no-unknown rule
at-rule-no-unknown: null
scss/at-rule-no-unknown: true
# Turn off rules enabled in stylelint-config-standard that conflict with SCSS
# syntax
annotation-no-unknown: null
at-rule-descriptor-no-unknown: null
at-rule-descriptor-value-no-unknown: null
at-rule-prelude-no-invalid: null
import-notation: null # We can remove this once all SCSS is using @use
declaration-property-value-no-unknown: null
media-query-no-invalid: null
function-no-unknown: null
media-feature-name-value-no-unknown: null
# General rules
# Collection of rules for if/else statements
scss/at-else-closing-brace-newline-after: always-last-in-chain
scss/at-else-closing-brace-space-after: always-intermediate
scss/at-else-empty-line-before: never
scss/at-if-closing-brace-newline-after: always-last-in-chain
scss/at-if-closing-brace-space-after: always-intermediate
# End collection of rules for if/else statements
scss/at-extend-no-missing-placeholder: true
scss/at-function-parentheses-space-before: never
scss/at-mixin-parentheses-space-before: never
5 changes: 4 additions & 1 deletion bin/run-markdownlint-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ for TEST_FILE in $MARKDOWN_TEST_FILES; do
fi
done


NUM_FILES=$(echo "$MARKDOWN_TEST_FILES" | wc -l | tr -d '[:space:]')

if [ "${TEST_DID_FAIL}" -eq 0 ]; then
echo "All markdownlint tests passed."
echo "All markdownlint tests passed across $NUM_FILES test file(s)."
else
exit 1
fi
28 changes: 28 additions & 0 deletions bin/run-stylelint-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/usr/bin/env bash

SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"

TEST_FILES=$(find "${SCRIPT_DIR}/../tests/stylelint" -type f -name '*.scss' | sort)

TEST_DID_FAIL=0

for TEST_FILE in $TEST_FILES; do
OUTPUT=$("${SCRIPT_DIR}/../node_modules/.bin/stylelint" "${TEST_FILE}" 2>&1)
EXIT_CODE=$?

if [[ "${TEST_FILE}" == *.pass.scss ]] && [ "${EXIT_CODE}" -ne 0 ]; then
echo "ERROR: Expected ${TEST_FILE} to pass linting, but it failed linting with:\n${OUTPUT}"
TEST_DID_FAIL=1
elif [[ "${TEST_FILE}" == *.fail.scss ]] && [ "${EXIT_CODE}" -eq 0 ]; then
echo "ERROR: Expected ${TEST_FILE} to fail linting, but the file passed linting"
TEST_DID_FAIL=1
fi
done

NUM_FILES=$(echo "$TEST_FILES" | wc -l | tr -d '[:space:]')

if [ "${TEST_DID_FAIL}" -eq 0 ]; then
echo "All stylelint tests passed across $NUM_FILES test file(s)."
else
exit 1
fi
Loading