[dev] Replace sass-lint with stylelint by jbudz · Pull Request #86177 · elastic/kibana

jbudz · 2020-12-16T19:45:00Z

sass-lint is unmaintained, so this replaces it with
an actively maintained linter. The changeset here includes two entry
points

node scripts/stylelint
via node scripts/register_precommit_hook on staged files

Closes #79323

sass-lint is unmaintained, so this replaces it with an actively maintained linter. The changeset here includes two entry points 1) node scripts/stylelint **/*.scss for general linting 2) via node scripts/register_precommit_hook on staged filed Lint rules and approach to migrating is TBD. I'm opening this now to showcase the code changes separately from a large lint changeset, and will circle back before merging depending on our approach.

.stylelintrc

This reverts commit 8311cd0.

elasticmachine · 2020-12-18T19:38:31Z

Pinging @elastic/kibana-operations (Team:Operations)

tylersmalley · 2020-12-21T19:21:43Z

.teamcity/src/builds/Lint.kt

@@ -12,12 +12,12 @@ object Lint : BuildType({



nit: also update the task description to replace sasslint with stylelint.

pushed with b4d9ab8

package.json

tylersmalley · 2020-12-21T19:24:46Z

scripts/stylelint.js


 require('../src/setup_node_env');
-require('../src/dev/run_sasslint');
+require('../src/dev/run_stylelint');


Running this script directly outputs the stylelint help.

Pushed d3cedd7. It uses the stylelint CLI parser to check for input globs, and if there aren't any we default to *.s(c|a)ss.

Co-authored-by: Tyler Smalley <tylersmalley@me.com>

jbudz · 2021-01-12T16:53:09Z

@elasticmachine merge upstream

jbudz · 2021-01-12T16:57:51Z

@snide I brought in most of the error rules from our sass-lint configurations. There were some missing analogues, e.g. border:none. Eyes from design would be helpful, we're targeting 7.12, so ~1 month if that's possible. I can push any review changes, or commits directly to this branch are fine.

snide · 2021-01-12T19:15:26Z

@jbudz I'll check this out later today. Thanks for doing the setup!

snide · 2021-01-13T20:39:55Z

Just an update here. This system works perefectly fine and I don't expect any changes on the engineering side. I'm adjusting some rules to be more strict and fixing sass as they fail. I should have a commit for this later today with a ✔️ .

snide · 2021-01-14T00:51:26Z

@jbudz PR for you. I added a couple dozen more rules and kicked the tires. Works great, and love that it can autofix. There's some more I'd like to add to this, but it can wait for a later PR when I have more time.

jbudz#2

Issues

At least in VIM, I noticed that the prettier autosave settings were conflicting with the style-lint settings. I'd like to have @cchaos or someone check to see what happens in VSCode. I see there are some other extensions we can run for prettier similar to what we're doing with ESLint, but after messing with them for a bit, I wasn't smart enough to get them running. I want make sure this isn't just something my VIM extensions are doing wrong with prettier first, but that may cause problems for this PR if we're seeing it in other editors.

clean up stylelint rules

jbudz · 2021-01-14T20:36:39Z

@snide I was able to reproduce the prettier/stylelint conflict. The most straightforward option is to disable prettier on .scss files (pushed with a951ded) and rely on stylelint for style and errors. We haven't had any enforcement on prettier formatting to date, so it's hit or miss if our files are formatted properly.

I looked at plugins - https://github.com/hugomrdias/prettier-stylelint looks like what we would want but it's not an active project.

The third option is to align our style changes with prettier's default. I'm not seeing a way to do the reverse, e.g. prettier/prettier#2705. This would involve migrating towards https://github.com/stylelint/stylelint-config-standard/ and a closer look at the prettier defaults.

Any thoughts?

snide · 2021-01-14T21:19:59Z

@jbudz I think that's a good option for now. People can use extensions in their editors for the style-lint warnings. Mostly wanted to make sure we didn't run into frustrations with people not being able to save files that style-lint liked.

snide

This looks OK from design. We'll add to the rules over time in separate PRs. Thanks for setting this up.

jbudz · 2021-01-15T15:19:58Z

@elasticmachine merge upstream

kibanamachine · 2021-01-15T17:42:20Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 44f61bd

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`advancedSettings`	919.6KB	919.6KB	-24.0B
`canvas`	1.6MB	1.6MB	+216.0B
`discover`	511.3KB	511.3KB	-12.0B
`enterpriseSearch`	1.8MB	1.8MB	-67.0B
`graph`	1.3MB	1.3MB	-28.0B
`home`	215.4KB	215.3KB	-132.0B
`indexManagement`	1.5MB	1.5MB	-4.0B
`kibanaLegacy`	146.9KB	146.9KB	-4.0B
`kibanaReact`	350.2KB	349.9KB	-320.0B
`lens`	1.1MB	1.1MB	+108.0B
`ml`	7.1MB	7.1MB	-1.3KB
`visTypeTable`	153.7KB	153.7KB	-12.0B
`visTypeVislib`	643.7KB	643.6KB	-76.0B
total			-1.7KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`data`	1003.8KB	1003.8KB	-8.0B
`embeddable`	233.2KB	233.3KB	+104.0B
`indexManagement`	129.1KB	129.1KB	+4.0B
`maps`	154.4KB	154.4KB	-16.0B
`mapsLegacy`	89.4KB	89.4KB	-4.0B
`transform`	24.5KB	24.5KB	-8.0B
`upgradeAssistant`	60.1KB	60.1KB	+29.0B
`visTypeTimeseries`	137.4KB	137.4KB	-8.0B
`visualizations`	170.3KB	170.1KB	-160.0B
total			-67.0B

History

💚 Build #98965 succeeded a951ded
💚 Build #98932 succeeded fd45086
💔 Build #98904 failed dc6128e
💔 Build #98874 failed 58d9bd8
💚 Build #98295 succeeded 7d23ff1

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Co-authored-by: Tyler Smalley <tylersmalley@me.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Dave Snider <dave.snider@gmail.com>

Co-authored-by: Tyler Smalley <tylersmalley@me.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Dave Snider <dave.snider@gmail.com> Co-authored-by: Tyler Smalley <tylersmalley@me.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Dave Snider <dave.snider@gmail.com>

jbudz · 2021-01-15T21:49:10Z

7.x: 1dc3276

jbudz added Team:Operations Kibana-Operations Team v8.0.0 v7.12.0 labels Dec 16, 2020

update ci scripts

17262cf

tylersmalley reviewed Dec 17, 2020

View reviewed changes

.stylelintrc Outdated Show resolved Hide resolved

jbudz added 11 commits December 18, 2020 11:52

Merge branch 'master' into stylelint

630ab0a

match all globs

12e2d04

add .stylelintignore

0ab0a60

add stylelint rules from eui

0b9fa70

ignore build and target folder

113cfb1

add globs for ci

17238ca

fix autofixable errors

98e5c10

include non-sass files

8311cd0

Revert "include non-sass files"

e053dbf

This reverts commit 8311cd0.

formatting

a5fc5a8

formatting

ddf474e

jbudz marked this pull request as ready for review December 18, 2020 19:38

jbudz requested review from a team as code owners December 18, 2020 19:38

jbudz added the release_note:skip Skip the PR/issue when compiling release notes label Dec 18, 2020

tylersmalley reviewed Dec 21, 2020

View reviewed changes

botelastic bot added the Feature:Embedding Embedding content via iFrame label Dec 21, 2020

tylersmalley reviewed Dec 21, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

tylersmalley reviewed Dec 21, 2020

View reviewed changes

jbudz and others added 4 commits December 21, 2020 15:03

Update package.json

96ff5e3

Co-authored-by: Tyler Smalley <tylersmalley@me.com>

update lint description

b4d9ab8

Merge branch 'master' into stylelint

c1e0e85

Merge branch 'master' into stylelint

e1a2d46

jbudz added 4 commits January 4, 2021 17:17

camelCase mixin, function, and variable names

073c863

remove duplicate text-align

5f00f34

block-closing-brace-newline-after

fda9109

selector-no-qualifying-type

52115fa

Merge branch 'master' into stylelint

7d23ff1

clean up stylelint rules

ca3be88

jbudz added 3 commits January 14, 2021 08:50

Merge pull request #2 from snide/stylerules

58d9bd8

clean up stylelint rules

update snapshot

dc6128e

Merge branch 'master' into stylelint

fd45086

jbudz force-pushed the stylelint branch from a4e6599 to fd45086 Compare January 14, 2021 16:58

disable prettier on scss files

a951ded

snide approved these changes Jan 14, 2021

View reviewed changes

Merge branch 'master' into stylelint

44f61bd

jbudz merged commit 51ba94d into elastic:master Jan 15, 2021

jbudz deleted the stylelint branch January 15, 2021 17:52

jbudz mentioned this pull request Jan 15, 2021

[7.x] [dev] Replace sass-lint with stylelint (#86177) #88498

Merged

jbudz added the backported label Jan 15, 2021

Conversation

jbudz commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Dec 18, 2020

Uh oh!

tylersmalley Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

jbudz Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tylersmalley Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

jbudz Dec 22, 2020

Choose a reason for hiding this comment

Uh oh!

jbudz commented Jan 12, 2021

Uh oh!

jbudz commented Jan 12, 2021

Uh oh!

snide commented Jan 12, 2021

Uh oh!

snide commented Jan 13, 2021

Uh oh!

snide commented Jan 14, 2021

Issues

Uh oh!

jbudz commented Jan 14, 2021

Uh oh!

snide commented Jan 14, 2021

Uh oh!

snide left a comment

Choose a reason for hiding this comment

Uh oh!

jbudz commented Jan 15, 2021

Uh oh!

kibanamachine commented Jan 15, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

jbudz commented Jan 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jbudz commented Dec 16, 2020 •

edited

Loading