Skip to content
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

Replace sass-lint with stylelint #949

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f557ebb
bump stylelint: 15.10.2 (#119)
gulderov Aug 3, 2023
7a9de5e
bump postcss: 8.4.27 (#119)
gulderov Aug 3, 2023
62460ed
stylelint configs (#119)
gulderov Aug 3, 2023
b6366ab
stylelint auto fix (#119)
gulderov Aug 3, 2023
a3b0e4c
remove stylelint config (#119)
gulderov Aug 3, 2023
7d80460
stylelint config cleanup (#119)
gulderov Aug 3, 2023
b226e9c
stylelint fix (#119)
gulderov Aug 3, 2023
4bb9391
stylelint fix src-docs (#119)
gulderov Aug 3, 2023
ae29559
stylelint fix (#119)
gulderov Aug 3, 2023
60a5680
sass-lint cleanup (#119)
gulderov Aug 3, 2023
ba7f671
scripts fix (#119)
gulderov Aug 3, 2023
417dda4
Merge branch 'main' into stylelint
gulderov Aug 3, 2023
d06bbaa
stylelint fix (#119)
gulderov Aug 3, 2023
e3b0ff6
removal of saas-lint comments (#119)
gulderov Aug 3, 2023
222f61b
limit script to lint:style only (#119)
gulderov Aug 4, 2023
c725ad3
prefer stylelint-disable-next-line over stylelint-disable-line (#119)
gulderov Aug 4, 2023
6d6e6f7
sass-lint cleanup (#119)
gulderov Aug 4, 2023
5154f25
rename scripts (#119)
gulderov Aug 4, 2023
b120590
add script lint-style-fix (#119)
gulderov Aug 4, 2023
0e5ba98
update changelog (#119)
gulderov Aug 4, 2023
945d2cb
quotes replace
gulderov Aug 8, 2023
16e2dad
quotes (#119)
gulderov Aug 9, 2023
870a9db
packages update (#119)
gulderov Aug 24, 2023
8f0d289
Merge branch 'main' into stylelint (#119)
gulderov Aug 24, 2023
ec4765c
Merge branch 'main' into stylelint
gulderov Aug 29, 2023
0904cb9
Merge branch 'main' into stylelint
gulderov Sep 6, 2023
d2eb2e1
stylelint fix (#119)
gulderov Sep 8, 2023
b07f171
Merge branch 'main' into stylelint
joshuarrrr Sep 13, 2023
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
12 changes: 0 additions & 12 deletions .sass-lint-fix.yml

This file was deleted.

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

This file was deleted.

2 changes: 2 additions & 0 deletions .stylelintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
build
target
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a few artifact directories:

dist
docs
es
lib
test-env
types

Additionally, would node_modules be a good choice to be here?

6 changes: 0 additions & 6 deletions .stylelintrc

This file was deleted.

19 changes: 19 additions & 0 deletions .stylelintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
extends:
- stylelint-config-standard-scss
rules:
# while we still use node-sass, only legacy rgb() notation is allowed
color-function-notation: "legacy"
Comment on lines +4 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

Will need to followup on this for #1001

# recommended to turn off descending specificity since we use a lot of nesting:
# https://stylelint.io/user-guide/rules/list/no-descending-specificity/
no-descending-specificity: null
Comment on lines +6 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to have a selector ordering rule. We should look into this further to see how much work it would be and if we should do it in the first place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could handle it as separate task

# need to use global function/value names from EUI
function-name-case: null
value-keyword-case: null
scss/no-global-function-names: null
# camelCase names
keyframes-name-pattern: "^[a-z][a-zA-Z0-9_-]+$"
selector-class-pattern: "^[a-z][a-zA-Z0-9_-]+$"
selector-id-pattern: "^[a-z][a-zA-Z0-9_-]+$"
scss/at-mixin-pattern: "^[a-z][a-zA-Z0-9_-]+$"
scss/at-function-pattern: "^[a-z][a-zA-Z0-9_-]+$"
scss/dollar-variable-pattern: "^[a-z][a-zA-Z0-9_-]+$"
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
- [CVE-2023-26115] Bump word-wrap from 1.2.3 to 1.2.4 ([#891](https://github.com/opensearch-project/oui/pull/891))
- Bump Node version to 18.16.0 ([#900](https://github.com/opensearch-project/oui/pull/900))
- Bump `node-sass` to a patched version based on `libsass@3.6.5` ([#977](https://github.com/opensearch-project/oui/pull/977)); see [patch commit](https://github.com/AMoo-Miki/node-sass/commit/43c74c0966b05c1e21a1e5e20a0c467ec8e669b4) for details.
- Replace `sass-lint` with `stylelint` ([#949](https://github.com/opensearch-project/oui/pull/949))

### 🪛 Refactoring

Expand Down
17 changes: 6 additions & 11 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
"clean": "node ./scripts/compile-clean.js",
"compile-icons": "node ./scripts/compile-icons.js && prettier --write --loglevel=warn \"./src/components/icon/assets/**/*.js\"",
"extract-i18n-strings": "node ./scripts/babel/fetch-i18n-strings",
"lint": "yarn tsc --noEmit && yarn lint-es && yarn lint-sass",
"lint": "yarn tsc --noEmit && yarn lint-es && yarn lint-style",
"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-style": "stylelint \"{src,src-docs}/**/*.scss\" --max-warnings 0",
"lint-style-fix": "yarn lint-style --fix",
Comment on lines +27 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious if this will cause anything unintended to break. All CI passes, but I wonder if anyone else is using these commands that we don't know about. Either way, I don't think it's something we really need to worry about for breaking things ~ this is more meant for dev anyway

"test": "yarn lint && yarn test-unit",
"test-unit": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.json",
"test-a11y": "node ./scripts/a11y-testing",
Expand Down Expand Up @@ -74,12 +74,6 @@
"react-view/**/minimist": "^1.2.6",
"react-view/@miksu/prettier/minimatch": "^3.0.8",
"remark-parse/trim": "0.0.3",
"sass-lint-auto-fix/**/minimist": "^1.2.6",
"sass-lint-auto-fix/merge": "^2.1.1",
"sass-lint/**/minimist": "^1.2.6",
"sass-lint/eslint": "^7.10.0",
"sass-lint/front-matter": "^4.0.2",
"sass-lint/merge": "^2.1.1",
"start-server-and-test/**/minimist": "^1.2.6",
"webpack-dev-server/**/ansi-regex": "^5.0.1",
"webpack-dev-server/chokidar/glob-parent": "^6.0.1",
Expand Down Expand Up @@ -216,6 +210,7 @@
"moment-timezone": "^0.5.41",
"node-sass": "npm:@amoo-miki/node-sass@9.0.0-libsass-3.6.5",
"pegjs": "^0.10.0",
"postcss": "^8.4.28",
Copy link
Contributor

Choose a reason for hiding this comment

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

I am astonished that we were using postcss without specifying it as a dep 🤦

"postcss-cli": "^7.1.2",
"postcss-inline-svg": "^4.1.0",
"postcss-loader": "^4.0.1",
Expand All @@ -237,13 +232,13 @@
"resolve": "^1.22.1",
"rimraf": "^5.0.1",
"sass-extract": "^2.1.0",
"sass-lint": "^1.13.1",
"sass-lint-auto-fix": "^0.21.2",
"sass-loader": "npm:@bsfishy/sass-loader@node-sass-9",
"sass-vars-to-js-loader": "^2.1.1",
"shelljs": "^0.8.5",
"start-server-and-test": "^2.0.0",
"style-loader": "^1.2.1",
"stylelint": "^15.10.3",
"stylelint-config-standard-scss": "^10.0.0",
"terser-webpack-plugin": "^4.1.0",
"typescript": "4.0.5",
"url-loader": "^4.1.0",
Expand Down
26 changes: 13 additions & 13 deletions src-docs/src/components/guide_components.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ $guideZLevelHighest: $ouiZLevel9 + 1000;
$elasticLogoTextLight: #FFF;
$elasticLogoTextDark: #1C1E23;

#guide { // sass-lint:disable-line no-ids
#guide {
display: flex;
flex-direction: column;
min-height: 100vh;
Expand All @@ -33,7 +33,7 @@ body {
.ouiBody--headerIsFixed--double {
@include ouiHeaderAffordForFixed($ouiHeaderHeightCompensation * 2);

#guide { // sass-lint:disable-line no-ids
#guide {
min-height: calc(100vh - #{$ouiHeaderHeightCompensation * 2});
}

Expand All @@ -46,11 +46,13 @@ body {

.guideSideNav {
@include ouiSideNavEmbellish;

min-width: $guideSideNavWidth;
}

.guideSideNav__content {
@include ouiYScroll;

padding: 0 $ouiSizeL $ouiSizeL;
}

Expand Down Expand Up @@ -100,7 +102,7 @@ body {
.guideDemo__highlightLayout--playground > div:not(.ouiPage) {
height: 100%;
width: 100%;
padding: 0 !important; // sass-lint:disable-line no-important
padding: 0 !important;

> .ouiPage--grow {
height: 100%;
Expand Down Expand Up @@ -137,14 +139,12 @@ body {
padding: $ouiSize;
}

// sass-lint:disable no-important
.guideDemo__textLines {
background-image: linear-gradient($ouiFocusBackgroundColor 1px, transparent 1px) !important;
background-size: 100% 8px !important;
background-position-y: 2px;
}

// sass-lint:disable no-important
.guideDemo__textLines--s {
background-image: linear-gradient($ouiFocusBackgroundColor 1px, transparent 1px) !important;
background-size: 100% 7px !important;
Expand Down Expand Up @@ -179,7 +179,7 @@ body {
}

.guideDemo__ghostBackground {
@if (lightness($ouiTextColor) < 50) {
@if lightness($ouiTextColor) < 50 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the extra space here? There seem to be a lot of these? Is it just an artifact of yarn lint-sass-fix?

Suggested change
@if lightness($ouiTextColor) < 50 {
@if lightness($ouiTextColor) < 50 {

Copy link
Contributor Author

@gulderov gulderov Aug 4, 2023

Choose a reason for hiding this comment

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

It is artifact from yarn lint-style --fix. I was wondering to fix it with prettier. But we need to update prettier first and that is separate task.

I tend to avoid manual intervention and keep it as is for now. Prettier would reformat it later

color: $ouiColorGhost;
background: $ouiColorDarkestShade !important; // Override OuiPanel specificity
}
Expand Down Expand Up @@ -247,6 +247,7 @@ body {

.guideDemo__notificationEvent {
@include ouiFontSizeS;

display: flex;
flex-direction: column;

Expand Down Expand Up @@ -324,10 +325,11 @@ body {
color: $ouiColorAccentText;
}


/* stylelint-disable no-invalid-position-at-import-rule */
@import '../views/guidelines/index';
@import 'guide_section/index';
@import 'guide_rule/index';
/* stylelint-enable no-invalid-position-at-import-rule */

@include ouiBreakpoint('xs', 's') {
.guideSideNav {
Expand All @@ -348,17 +350,16 @@ body {
min-width: 200px;
flex-basis: 100% !important;

// sass-lint:disable-block mixins-before-declarations
@include ouiBreakpoint('s', 'm') {
flex-basis: 45% !important; // sass-lint:disable-line no-important
flex-basis: 45% !important;
}

@include ouiBreakpoint('l') {
flex-basis: 30% !important; // sass-lint:disable-line no-important
flex-basis: 30% !important;
}

@include ouiBreakpoint('xl') {
flex-basis: 22% !important; // sass-lint:disable-line no-important
flex-basis: 22% !important;
}
}

Expand All @@ -383,7 +384,7 @@ body {
width: 100%;
height: auto;

&:before {
&::before {
content: '';
display: block;
position: absolute;
Expand Down Expand Up @@ -458,7 +459,6 @@ body {
}

.guideHome__footerLogo {
// sass-lint:disable-block no-important
vertical-align: middle;
display: inline-block !important;
margin-bottom: 0 !important;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@
}

.guideSectionTabs__tab {
font-weight: $ouiFontWeightMedium !important; // sass-lint:disable-line no-important
font-weight: $ouiFontWeightMedium !important;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@

.playgroundWrapper {
> div {
display: block !important; // sass-lint:disable-line no-important
display: block !important;
}
}
5 changes: 3 additions & 2 deletions src-docs/src/theme_dark.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
* GitHub history for details.
*/

// sass-lint:disable no-url-domains, no-url-protocols
/* stylelint-disable-next-line import-notation */
@import url('https://fonts.googleapis.com/css?family=Roboto+Mono:400,400i,700,700i');
@import url('https://rsms.me/inter/inter-ui.css');

/* stylelint-disable-next-line import-notation */
@import url('https://rsms.me/inter/inter-ui.css');
@import '../../src/theme_dark';
@import './components/guide_components';
@import './services/playground/index';
Expand Down
5 changes: 3 additions & 2 deletions src-docs/src/theme_light.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
* GitHub history for details.
*/

// sass-lint:disable no-url-domains, no-url-protocols
/* stylelint-disable-next-line import-notation */
@import url('https://fonts.googleapis.com/css?family=Roboto+Mono:400,400i,700,700i');
@import url('https://rsms.me/inter/inter-ui.css');

/* stylelint-disable-next-line import-notation */
@import url('https://rsms.me/inter/inter-ui.css');
@import '../../src/theme_light';
@import './components/guide_components';
@import './services/playground/index';
Expand Down
3 changes: 1 addition & 2 deletions src-docs/src/theme_next_dark.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
* GitHub history for details.
*/

// sass-lint:disable no-url-domains, no-url-protocols
/* stylelint-disable-next-line import-notation */
@import url('https://fonts.googleapis.com/css2?family=Source+Code+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&family=Source+Sans+3:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap');

@import '../../src/theme_next_dark';
@import './components/guide_components';
@import './services/playground/index';
Expand Down
3 changes: 1 addition & 2 deletions src-docs/src/theme_next_light.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
* GitHub history for details.
*/

// sass-lint:disable no-url-domains, no-url-protocols
/* stylelint-disable-next-line import-notation */
@import url('https://fonts.googleapis.com/css2?family=Source+Code+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&family=Source+Sans+3:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap');

@import '../../src/theme_next_light';
@import './components/guide_components';
@import './services/playground/index';
Expand Down
Loading