Lint: use @wordpress/stylelint-config preset

[manzoorwanijk]

Proposed Changes

This PR updates .stylelintrc to use @wordpress/stylelint-config preset and removes certain rules that are already provided by @wordpress/stylelint-config.

Testing Instructions

It is assumed that you use VS Code for testing

• Ensure that you have the official Stylelint extension installed and active.
• Checkout this PR
• Ensure that you have these settings in settings.json (Cmd + Shift + P > Preferences: Open Settings (JSON))

	"stylelint.validate": ["css", "scss", "sass"],
	"[css][scss][sass]": {
		"editor.formatOnSave": false,
		"editor.defaultFormatter": "stylelint.vscode-stylelint"
	},
	"editor.codeActionsOnSave": {
		"source.fixAll.stylelint": true
		// other actions
	},

• Reload VS Code: Cmd + Shift + P > Developer: Reload Window
• Open a SCSS file e.g. client/me/security-2fa-initial-setup/style.scss
• Mess up the formatting e.g. add empty lines, use spaces for indentation, etc.
• Open problems view (Cmd + Shift + P > Problems: Focus on Problems View)
• Confirm that you see the problems reported by Stylelint.
• Save Changes (Cmd + S)
• Confirm that the file is formatted as expected.

Pre-merge Checklist

• Have you written new tests for your changes?
• Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
• Have you checked for TypeScript, React or other console errors?
• Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
• Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?

Related to #39568

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-41441

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-41441&env=jetpack

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[noahtallen]

Nice, thanks for looking at this! I think we might need to tweak the rules a bit more. I think we should add these:

"rule-empty-line-before": [ "always", { "ignore": [ "after-comment", "first-nested" ] } ],
"at-rule-empty-line-before": [ "always", { "ignore": [ "after-comment", "first-nested" ] } ],

If you look at a file like apps/editing-toolkit/editing-toolkit-plugin/common/index.scss, you'll see some weird errors until you add the above. (I don't think the above rules conflict with the WordPress style guidelines either, afaik.)

Open to other ideas too

[manzoorwanijk]

I think we might need to tweak the rules a bit more. I think we should add these:

"rule-empty-line-before": [ "always", { "ignore": [ "after-comment", "first-nested" ] } ],
"at-rule-empty-line-before": [ "always", { "ignore": [ "after-comment", "first-nested" ] } ],

I did notice that behavior. I thought maybe we want those empty lines. But yeah, I agree that we should try to minimize the number of lints for now.

I have made the changes. Thank you.

[tyxla]

I'm getting this error when trying to use the yarn run lint:css command:

Error: The "syntax" option is no longer available. You should install an appropriate syntax, e.g. postcss-scss, and use the "customSyntax" option
    at getPostcssResult (/Users/marin/Projects/calypso/node_modules/stylelint/lib/getPostcssResult.js:37:25)
    at lintSource (/Users/marin/Projects/calypso/node_modules/stylelint/lib/lintSource.js:79:20)
    at async /Users/marin/Projects/calypso/node_modules/stylelint/lib/standalone.js:228:27
    at async Promise.all (index 4)
    at async standalone (/Users/marin/Projects/calypso/node_modules/stylelint/lib/standalone.js:267:22)

[manzoorwanijk]

I'm getting this error when trying to use the yarn run lint:css command:

@tyxla Thank you for catching that. It was due to a migration needed after upgrading stylelint to v14. I have added the required config change and it's good now.

[tyxla]

Thanks for fixing that @manzoorwanijk! However, there's this warning when running yarn install now:

➤ YN0002: │ stylelint-config-recommended-scss@npm:7.0.0 [676fa] doesn't provide postcss (pdadd3), requested by postcss-scss

and the Unit Tests check appears to be failing because of that as well.

[noahtallen]

@manzoorwanijk I pushed a commit fixing the peer dependency issues -- these often happen when updating WordPress dependencies and can be a bit tricky to resolve. (Especially since the WordPress dependencies are so inter-dependent, we may have to add multiple peer dependency rules to match the different dependency versions when we don't update them all at the same time.)

[manzoorwanijk]

I pushed a commit fixing the peer dependency issues

Thank you for that @noahtallen

[noahtallen]

Looking through a few more files, I think we could make some more rule changes. For example, in packages/help-center/src/styles.scss, I see these issues:

• Indentation is converted to spaces instead of tabs
• The @import statements at the top get newlines put between them
• there are some rules about specificity and kebab-case we should double check

I just want to make sure we don't introduce any weird patterns once people start using this and turn auto-fixing on :)

[manzoorwanijk]

Looking through a few more files, I think we could make some more rule changes. For example, in packages/help-center/src/styles.scss, I see these issues:

• Indentation is converted to spaces instead of tabs
• The @import statements at the top get newlines put between them
• there are some rules about specificity and kebab-case we should double check

I just want to make sure we don't introduce any weird patterns once people start using this and turn auto-fixing on :)

@noahtallen the issue was the order of extended configs in extends with subsequent ones overriding the previous ones.

Fixed that along with empty lines issue in imports.

[noahtallen]

Cool, I think this is good to merge. Thanks for working on it!

[noahtallen]

@manzoorwanijk Why did we need to extend "stylelint-config-standard-scss"?

[manzoorwanijk]

Why did we need to extend "stylelint-config-standard-scss"?

Because in v14, stylelint removed the built-in support for custom syntaxes and only supports CSS by default. Now, in order to use SCSS, we need specific config which is provided by "stylelint-config-standard-scss".

More details here https://stylelint.io/migration-guide/to-14/

[simison]

@manzoorwanijk could that be something done/changed in upstream package so that Calypso doesn't need to take that into account when re-using the upstream confirg?

[manzoorwanijk]

could that be something done/changed in upstream package so that Calypso doesn't need to take that into account when re-using the upstream confirg?

I think I didn't realize that @wordpress/stylelint-config extends stylelint-config-recommended-scss which in turn does everything that stylelint-config-standard-scss does. So, it looks like we can simply remove extending stylelint-config-standard-scss.

CC: @noahtallen

[simison]

A good next steps to sort out all the new linting failures would be:

• Ensure Prettier autoformatting works with the new setup
• Add easy-to-repeat autoformatter script, feel free to take over Add reformat-files:css script #32351
• Run and commit fixes, either one big PR or many smaller ones
• Turn on CI failure on linting errors once we're 0 errors, or if we can't get to it easily, fail CI if the number increases.

Thanks for everyone's work on this! Great to see this taken care finally.

[manzoorwanijk]

A good next steps to sort out all the new linting failures would be:

• Ensure Prettier autoformatting works with the new setup
• Add easy-to-repeat autoformatter script, feel free to take over Add reformat-files:css script #32351
• Run and commit fixes, either one big PR or many smaller ones
• Turn on CI failure on linting errors once we're 0 errors, or if we can't get to it easily, fail CI if the number increases.

Thanks for everyone's work on this! Great to see this taken care finally.

I think @noahtallen has already started working on it in #67468 🚀