Skip to content

Commit

Permalink
Replace sass-lint dependency with stylelint (#6470)
Browse files Browse the repository at this point in the history
* Add `stylelint` and scss configs

- see https://github.com/stylelint/stylelint/blob/main/docs/user-guide/get-started.md#using-a-community-shared-config

- note that adding postcss@8 is necessary for the scss config to work - see stylelint-scss/stylelint-config-standard-scss#5

* Convert `.stylelintrc` to `.stylelintrc.js` + add scss configs

- JS file helps us do things like leave meaningful comments, which JSON doesn't allow

* Update ignored files

- previously ignored files were added due to vscode's overeager linting - it looks like said plugin no longer does so, so we can remove non `.scss` files

- port over sass-lint excludes
  - add missed datepicker dir
  - add non-src files
  - remove no-longer-existant chart dir

* Remove sass-lint, swap over yarn shortcuts

* Convert camelCase name linting

* Convert indentation lint rule
+ fix issues caught by stylelint but not by sass-lint

- unlike sass-lint, stylelint checks indentation of comments and also checks for tabs vs spaces

+ fix a few unnecessary `()`s, and add a disable because stylelint is confused by scss maps

+ remove unnecessary indentation disables

* Convert single quotes lint rule
+ add skip for Sass scenarios where `"` has to be used for interpolation

* Convert brace style lint rule

- there isn't a handy `1tbs` option like eslint/sass-lint, so we need to combine a few different rules to get the effecrt we want

+ fix various instances where stylelint caught unnecessary double spaces before braces

* Convert various value lint preferences

+ include some rules that stylelint is more strict about than sass-lint was

* Fix pseudo-element lint rule

- sass-lint was incorrectly marking pseudo *classes* as needing two `::`, whereas only pseudo *elements* need two `::`s. stylelint appears to correctly catch this OOTB

@see https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Selectors/Pseudo-classes_and_pseudo-elements

* Convert newline between rulesets/blocks lint rule

* [opinionated] Add several new lint rules catching extra newlines/whitespace

- prettier does the same/similar for JS/CSS-in-JS in any case

* Add lint rule for 1 selector per line

- already appears to be something we do or was enforced by sass-lint, but stylelint needs the explicit rule

* Convert/fix rules around syntax whitespaces

* [opinionated] Fix several comment rules stylelint ships with OOTB

- specifically around empty comments & spacing around comments - I find them to be valid and inline with JS linting

* Fix `no-duplicate-selectors` caught by stylelint (but not sass-lint)

* Override default stylelint rules for  backwards-compatability

- without these added rules, we get stylelint errors for how we currently write our CSS

- although we may want to consider removing these in the future & using more modern syntax

* Fix vendor prefix linting by converting to stylelint-disable syntax

* Disable various opinionated stylelist defaults

- that were not previous enforced by sass-lint

- we can consider turning them on at a later time if we want

* Convert no `!important` rule and overrides

+ remove unnecessary disables

* Convert nesting depth rule

- default with sass-lint but not with stylelint or existing configs

+ remove unnecessary disables

+ add newly necessary disables (some it's unclear why sass-lint didn't catch, and some are due to sass syntax, which stylelint unfortunately does not care about)

* Convert Sass empty ruleset lint rule

- by default, stylelint considers comments to not make the block empty

* Convert qualifying elements selector lint rule

* Clarify function name lint rule

+ remove disables

* Attempt to convert

- this is not perfect - stylelint does not appear to have a 1:1 with sass-lint on this, but it's somewhat close and will become less of an issue as we migrate to Emotion

+ remove disable lines that no longer apply

* Remove unnecessary sass lint disables

- no-mispelled-properties is simply wrong / out of date

- no-warn was already disabled in -sass-lint.yml

- empty-args isn't useful in a post-sass world

* 🚫 Remove mixin order rule

- there's no corresponding stylelint rule, and it feels pedantic / not terribly useful

* 🚫 Remove border 0 rule

- there's no corresponding stylelint rule, and it feels pedantic / not useful

* Remove all remaining disabled sass-lint rules

- these don't appear to be enforced by stylelint, and as such nothing needs to be done

* Delete all sass lint config files

* Add some more stylelint docs links & notes

* Remove `prettier-stylelint`

- It's not clear to me why that plugin was added or what it does, and it was last updated 5 years ago and uses a super old version of stylelint (8.x), so I'm removing it

* [PR feedback] Remove `number-leading-zero` rule

- thanks Elizabet!
  • Loading branch information
Cee authored Dec 12, 2022
1 parent 71e3bfb commit 0d31625
Show file tree
Hide file tree
Showing 116 changed files with 738 additions and 1,698 deletions.
12 changes: 0 additions & 12 deletions .sass-lint-fix.yml

This file was deleted.

89 changes: 0 additions & 89 deletions .sass-lint.yml

This file was deleted.

13 changes: 0 additions & 13 deletions .stylelintrc

This file was deleted.

119 changes: 119 additions & 0 deletions .stylelintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
const camelCaseRegex = '^[a-z][\\w-]*$'; // Note: also allows `_` as part of BEM naming

// TODO: Remove Sass-specific config & rules once we're completely off Sass
module.exports = {
extends: ['stylelint-config-standard-scss', 'stylelint-config-prettier-scss'],
// @see https://stylelint.io/user-guide/rules
// @see https://github.com/stylelint-scss/stylelint-scss#list-of-rules
rules: {
// Enforce camelCase naming
'selector-class-pattern': camelCaseRegex,
'keyframes-name-pattern': camelCaseRegex,
'scss/dollar-variable-pattern': camelCaseRegex,
'scss/at-mixin-pattern': camelCaseRegex,
'scss/at-function-pattern': camelCaseRegex,
// TODO: This rule can be removed entirely (already lower by default) once we're fully off Sass
'function-name-case': [
'lower',
{
ignoreFunctions: [`/${camelCaseRegex}/`, 'MIN'],
},
],

// Opinionated rules
'declaration-no-important': true,
'max-nesting-depth': [
2,
{
ignore: ['blockless-at-rules', 'pseudo-classes'],
},
],
'block-no-empty': true,
'selector-no-qualifying-type': [
true,
{
ignore: ['attribute'], // Allows input[type=search]
},
],

// 2 spaces for indentation
indentation: [
2,
{
indentInsideParens: 'once-at-root-twice-in-block',
},
],
'string-quotes': 'single',
// Mimic 1tbs `} else {` brace style, like our JS
'block-opening-brace-space-before': 'always',
'block-closing-brace-newline-before': 'always-multi-line',
'scss/at-if-closing-brace-space-after': 'always-intermediate',
// Put a line-break between sections of CSS, but allow quick one-liners for legibility
'rule-empty-line-before': [
'always-multi-line',
{
except: ['first-nested'],
ignore: ['after-comment'],
},
],
// Ensure multiple selectors on one line each
'selector-list-comma-newline-before': 'never-multi-line',
'selector-list-comma-newline-after': 'always',
// Trim unnecessary newlines/whitespace
'block-closing-brace-empty-line-before': 'never',
'declaration-empty-line-before': null,
'max-empty-lines': 1,
'no-eol-whitespace': true,
// Enforce spacing around various syntax symbols (colons, operators, etc.)
'declaration-colon-space-after': 'always-single-line',
'declaration-colon-space-before': 'never',
'function-calc-no-unspaced-operator': true,
'scss/operator-no-unspaced': true,
'selector-combinator-space-before': 'always',
'selector-combinator-space-after': 'always',
// Ensure trailing semicolons are always present on non-oneliners
'declaration-block-semicolon-newline-after': 'always-multi-line',

// Value preferences
'number-max-precision': null,
'color-hex-case': 'upper',
// Attempt to catch/flag non-variable color values
'color-named': 'never',
'color-no-hex': true,
// Prefer lowercase values, except for font names and currentColor
'value-keyword-case': [
'lower',
{
ignoreProperties: ['font-family', '/^\\$eui[\\w]+/'], // Allow fonts and Sass variables
ignoreKeywords: ['currentColor'],
},
],
'declaration-block-no-duplicate-properties': [
true,
{
ignore: ['consecutive-duplicates'], // We occasionally use duplicate property values for cross-browser fallbacks
},
],

// TODO: It may be worth investigating and updating these rules to their more modern counterparts
'selector-not-notation': 'simple',
'color-function-notation': 'legacy',
'alpha-value-notation': 'number',

// Disable various opinionated extended stylelint rules that EUI has not previously enforced
'no-descending-specificity': null,
'keyframe-selector-notation': null,
'declaration-block-no-redundant-longhand-properties': null,
'scss/no-global-function-names': null,
'scss/dollar-variable-empty-line-before': null,
'scss/at-rule-conditional-no-parentheses': null,
'scss/double-slash-comment-empty-line-before': null,
'scss/at-if-no-null': null,
},
ignoreFiles: [
'generator-eui/**/*.scss',
'src/global_styling/react_date_picker/**/*.scss',
'src/themes/amsterdam/global_styling/react_date_picker/**/*.scss',
'src/components/date_picker/react-datepicker/**/*.scss',
],
};
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
"lint-fix": "yarn lint-es-fix",
"lint-es": "eslint --cache \"{src,src-docs}/**/*.{ts,tsx,js}\" --max-warnings 0",
"lint-es-fix": "yarn lint-es --fix",
"lint-sass": "sass-lint -v --max-warnings 0",
"lint-sass-fix": "sass-lint-auto-fix -c ./.sass-lint-fix.yml",
"lint-sass": "yarn stylelint \"**/*.scss\"",
"lint-sass-fix": "yarn stylelint \"**/*.scss\" --fix",
"test": "yarn lint && yarn test-unit",
"test-ci": "yarn test && yarn test-cypress",
"test-unit": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.json",
Expand Down Expand Up @@ -208,12 +208,12 @@
"node-sass": "^6.0.1",
"path": "^0.12.7",
"pegjs": "^0.10.0",
"postcss": "^8.4.19",
"postcss-cli": "^7.1.2",
"postcss-inline-svg": "^4.1.0",
"postcss-loader": "^7.0.1",
"pre-commit": "^1.2.2",
"prettier": "^2.1.2",
"prettier-stylelint": "^0.4.2",
"process": "^0.11.10",
"prompt": "^1.0.0",
"prop-types": "^15.6.0",
Expand All @@ -233,12 +233,13 @@
"redux-thunk": "^2.3.0",
"resolve": "^1.17.0",
"rimraf": "^3.0.2",
"sass-lint": "^1.13.1",
"sass-lint-auto-fix": "^0.21.2",
"sass-loader": "^13.0.2",
"shelljs": "^0.8.4",
"start-server-and-test": "^1.11.3",
"style-loader": "^3.3.1",
"stylelint": "^14.15.0",
"stylelint-config-prettier-scss": "^0.0.1",
"stylelint-config-standard-scss": "^6.1.0",
"terser-webpack-plugin": "^5.3.5",
"typescript": "4.5.3",
"uglifyjs-webpack-plugin": "^2.2.0",
Expand Down
3 changes: 1 addition & 2 deletions src-docs/src/components/guide_page/_guide_page.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

.guideBody {
// Override euiHeaderAffordForFixed mixin since the page template handles this now
padding-top: 0 !important; // sass-lint:disable-line no-important
padding-top: 0 !important; // stylelint-disable-line declaration-no-important
}

.euiBody--headerIsFixed--double {
Expand Down Expand Up @@ -60,7 +60,6 @@
color: $euiTitleColor;
}


@include euiBreakpoint('xs', 's') {
.guideSideNav {
width: 100%;
Expand Down
2 changes: 0 additions & 2 deletions src-docs/src/components/guide_rule/_guide_rule.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,3 @@
margin-top: 0;
}
}


4 changes: 2 additions & 2 deletions src-docs/src/components/guide_section/_guide_section.scss
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
}

.guideSectionTabs__tab > * {
font-weight: $euiFontWeightMedium !important; // sass-lint:disable-line no-important
font-weight: $euiFontWeightMedium !important; // stylelint-disable-line declaration-no-important
}

.guideDemo__ghostBackground {
@if (lightness($euiTextColor) < 50) {
color: $euiColorGhost;
// Override EuiPanel specificity
background: $euiColorDarkestShade !important; // sass-lint:disable-line no-important
background: $euiColorDarkestShade !important; // stylelint-disable-line declaration-no-important
}
}
1 change: 0 additions & 1 deletion src-docs/src/services/playground/_playground_knobs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@ $knobTableRowMinHeight: 42px;
min-height: $knobTableRowMinHeight;
display: block;
}

4 changes: 2 additions & 2 deletions src-docs/src/views/datagrid/_datagrid.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
.euiDataGridRowCell--favoriteFranchise {
background: transparentize($color: #800080, $amount: .95) !important; // sass-lint:disable-line no-important
background: transparentize($color: #800080, $amount: .95) !important; // stylelint-disable-line declaration-no-important, color-no-hex
}

.euiDataGridHeaderCell--favoriteFranchise {
background: transparentize($color: #800080, $amount: .9) !important; // sass-lint:disable-line no-important
background: transparentize($color: #800080, $amount: .9) !important; // stylelint-disable-line declaration-no-important, color-no-hex
}

.euiDataGridRow--rowClassesDemo {
Expand Down
4 changes: 2 additions & 2 deletions src-docs/src/views/empty_prompt/_empty_prompt.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.guideDemo__emptyPromptPanelPicker {
width: 100%;
gap: $euiSizeXL; // sass-lint:disable-line no-misspelled-properties
gap: $euiSizeXL;
}

.guideDemo__emptyPromptPanelPickerThumbBtn {
Expand All @@ -26,4 +26,4 @@
.guideDemo__emptyPromptDemoPreview {
border: 1px solid $guideDemoHighlightColor;
overflow: hidden;
}
}
Loading

0 comments on commit 0d31625

Please sign in to comment.