Scripts: Update stylelint dependency and the default config #64828

mikeybinns · 2024-08-27T12:37:05Z

What?

Updating the required version of Stylelint to the latest version, and ensuring we aren't removing any rules which were migrated to a new package.

Why?

Keeping the Stylelint dependency up to date is important so we continue to get support and related tooling continues to work.
It's also important that the WordPress config remains the same so there aren't issues with CSS across the large codebase.

How?

I've updated the stylelint peer dependency from ^14.2 to ^16.8.2 in the @wordpress/stylelint-config package
I've updated the stylelint-config-recommended dependency from ^6.0.0 to ^14.0.1 in the @wordpress/stylelint-config package
I've updated the stylelint-config-recommended-scss dependency from ^5.0.2 to ^14.1.0 in the @wordpress/stylelint-config package
I've added the @stylistic/stylelint-config dependency at version ^2.0.0 in the @wordpress/stylelint-config package
I've migrated all stylistic rules to the @stylistic namespace in configs and disable/enable comments
I've moved all stylistic rules into new configs so it is easy for consumers to opt out of stylistic rules instead of using something like stylelint-config-prettier to disable them.
I fixed a bug with the declaration-block-no-duplicate-properties rule where it was being overwritten by the stylelint-config-recommended-scss config in the scss config.
I've disabled all newly added rules, even if they would have been added in the recommended configs to make this migration easier (we can add new rules back later if deemed necessary in a MINOR release)
I've migrated scss/at-import-partial-extension to scss/load-partial-extension as the former was deprecated.

Once the @wordpress/stylelint-config package was updated, I had to make some changes to the @wordpress/scripts package and the root stylelint config as the scripts package directly uses the stylelint config, and scripts is used to lint all CSS in the monorepo using the root config.

I updated the Stylelint dependency from ^14.2 to ^16.8.2
I updated the scripts default config to use @wordpress/stylelint-config/scss-stylistic to keep using the stylistic rules
I updated the monorepo stylelint config to use @wordpress/stylelint-config/scss-stylistic and migrate and rule overrides to the new rule names.

Testing Instructions

Run npm run lint:css to run the linting against the monorepo
Run npm run test:unit -- --testPathPattern packages/stylelint-config to run the automated tests in only this package

Fixes: #48142
Fixes: #54127
Closes: #48143
Closes: #50336
Closes: #48769
Closes: #51192
Closes: #54935

…istic

…ut for consumers

github-actions · 2024-08-27T12:37:20Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @75th, @vicobot-0815.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: 75th, vicobot-0815.

Co-authored-by: mikeybinns <mikeybinns@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: jacobcassidy <jacobcassidy@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: paulschreiber <paulschreiber@git.wordpress.org>
Co-authored-by: stein2nd <stein2nd@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

mikeybinns · 2024-08-27T13:27:03Z

Converting to draft while I fix Stylelint unit tests :)

mikeybinns · 2024-08-27T23:01:42Z

@gziolo I'm struggling with fixing these tests.

It seems that as of v16, Stylelint in Jest must be run in the node environment to work in Jest (source: stylelint/stylelint#7894) as the presence of a document breaks things.

I tried running the following command to test if that fix worked:

npm run test:unit -- --testPathPattern packages/stylelint-config --env=node

but this encounters a new error around global setup in jest-preset-default.

If I delete the contents of the packages/jest-preset-default/scripts/setup-globals.js and test/unit/config/global-mocks.js files, the tests proceed as expected (snapshot failures).

My jest knowledge isn't very good, and I certainly don't want to go messing around with the jest config, so would you be able to help me out with this? Perhaps there's a way to define a jest config just for this package?

I've pushed all my changes so you can pull down my branch.

gziolo · 2024-08-28T10:14:33Z

https://jestjs.io/docs/configuration#testenvironment-string

Have you tried changing the test env for these test files in the package? That would be the correct environment anyway there. A code comment at the top of every failing test suite file should do the trick.

/**
 * @jest-environment node
 */

All the mocks that depend on window can be guarded with the if check to account for the node environment where window is undefined anyway. For example:

diff --git a/packages/jest-preset-default/scripts/setup-globals.js b/packages/jest-preset-default/scripts/setup-globals.js
index 37922a3302..242d525936 100644
--- a/packages/jest-preset-default/scripts/setup-globals.js
+++ b/packages/jest-preset-default/scripts/setup-globals.js
@@ -2,56 +2,63 @@
 // eslint-disable-next-line @wordpress/wp-global-usage
 globalThis.SCRIPT_DEBUG = true;
 
-// These are necessary to load TinyMCE successfully.
-global.URL = window.URL;
-global.window.tinyMCEPreInit = {
-	// Without this, TinyMCE tries to determine its URL by looking at the
-	// <script> tag where it was loaded from, which of course fails here.
-	baseURL: 'about:blank',
-};
-
-global.window.setImmediate = function ( callback ) {
-	return setTimeout( callback, 0 );
-};
-
-global.window.requestAnimationFrame = function requestAnimationFrame(
-	callback
-) {
-	// eslint-disable-next-line no-restricted-syntax
-	const randomDelay = Math.round( ( Math.random() * 1_000 ) / 60 );
-
-	return setTimeout( () => callback( Date.now() ), randomDelay );
-};
-
-global.window.cancelAnimationFrame = function cancelAnimationFrame( handle ) {
-	return clearTimeout( handle );
-};
-
-// Ignoring `options` argument since we unconditionally schedule this ASAP.
-global.window.requestIdleCallback = function requestIdleCallback( callback ) {
-	const start = Date.now();
-
-	return setTimeout(
-		() =>
-			callback( {
-				didTimeout: false,
-				timeRemaining: () => Math.max( 0, 50 - ( Date.now() - start ) ),
-			} ),
-		0
-	);
-};
-
-global.window.cancelIdleCallback = function cancelIdleCallback( handle ) {
-	return clearTimeout( handle );
-};
-
-global.window.matchMedia = () => ( {
-	matches: false,
-	addListener: () => {},
-	addEventListener: () => {},
-	removeListener: () => {},
-	removeEventListener: () => {},
-} );
-
-// UserSettings global.
-global.window.userSettings = { uid: 1 };
+if ( typeof window !== 'undefined' ) {
+	// These are necessary to load TinyMCE successfully.
+	global.URL = window.URL;
+	global.window.tinyMCEPreInit = {
+		// Without this, TinyMCE tries to determine its URL by looking at the
+		// <script> tag where it was loaded from, which of course fails here.
+		baseURL: 'about:blank',
+	};
+
+	global.window.setImmediate = function ( callback ) {
+		return setTimeout( callback, 0 );
+	};
+
+	global.window.requestAnimationFrame = function requestAnimationFrame(
+		callback
+	) {
+		// eslint-disable-next-line no-restricted-syntax
+		const randomDelay = Math.round( ( Math.random() * 1_000 ) / 60 );
+
+		return setTimeout( () => callback( Date.now() ), randomDelay );
+	};
+
+	global.window.cancelAnimationFrame = function cancelAnimationFrame(
+		handle
+	) {
+		return clearTimeout( handle );
+	};
+
+	// Ignoring `options` argument since we unconditionally schedule this ASAP.
+	global.window.requestIdleCallback = function requestIdleCallback(
+		callback
+	) {
+		const start = Date.now();
+
+		return setTimeout(
+			() =>
+				callback( {
+					didTimeout: false,
+					timeRemaining: () =>
+						Math.max( 0, 50 - ( Date.now() - start ) ),
+				} ),
+			0
+		);
+	};
+
+	global.window.cancelIdleCallback = function cancelIdleCallback( handle ) {
+		return clearTimeout( handle );
+	};
+
+	global.window.matchMedia = () => ( {
+		matches: false,
+		addListener: () => {},
+		addEventListener: () => {},
+		removeListener: () => {},
+		removeEventListener: () => {},
+	} );
+
+	// UserSettings global.
+	global.window.userSettings = { uid: 1 };
+}

gziolo · 2024-08-28T10:26:04Z

packages/stylelint-config/index.js

@@ -1,7 +1,7 @@
 'use strict';

 module.exports = {
-	extends: 'stylelint-config-recommended',
+	extends: [ 'stylelint-config-recommended' ].map( require.resolve ),


What's the reason for using map here` with a single item?

I made it an array when I was previously using @stylistic/stylelint-config, but when I removed that config, I kept it as an array.

There was then a second error where it wouldn't find the config, so I fixed that with map/require.resolve as that's how the other configs are set up.

I can convert this to a non-mapped version if you'd prefer that?

I see it is used in other places with multiple items. It's fine.

gziolo · 2024-08-28T10:28:28Z

I mostly skimmed the changes applied and it all looks like everything is finally on the right track. The most important part is that linting passes! Excellent work 👏🏻

mikeybinns · 2024-08-28T11:31:51Z

https://jestjs.io/docs/configuration#testenvironment-string

Have you tried changing the test env for these test files in the package? That would be the correct environment anyway there. A code comment at the top of every failing test suite file should do the trick.
/**
 * @jest-environment node
 */

Ah, yes I had tried this, but adding a eslint-disable rule about the tag being incorrect stopped it from working as it was no longer at the top of the file. This time, I tried adding an exclusion to the rule in the eslint config (just added to the monorepo root config) and it worked 👍

mikeybinns · 2024-08-28T12:15:46Z

@gziolo As expected, once I sorted the global jest things to be wrapped in window checks, I run into this error:

Stylelint is now an ESM package as of v16, which Jest doesn't like.

I tried converting only the test files to ESM, and then adding that flag to the test:unit command in the root package.json, but I now get this different error:

It's not always 'stylelint', sometimes it's other files e.g. request for './hasLessInterpolation.js' is not yet fulfilled

From what I can see, it's an issue with jest-runtime and we might be SOL unless you have any ideas.
The Jest v30 alpha 1 has this line:

[jest-runtime] Properly handle re-exported native modules in ESM via CJS

This indicates to me that maybe the new Jest version will fix this issue?

Note: I've pushed all my code so far again.

mikeybinns · 2024-08-29T00:05:06Z

@gziolo Actually, don't worry. I've found a way to use node exec to use the CLI to bypass using the node API entirely, and still get the same result. Probably is a bit clunky compared to using the API, but at least it works without having to rethink the whole jest setup 🥳

mikeybinns · 2024-09-06T22:12:02Z

Copying the directory would work, but a cleaner way might be to use npm link.
Use Git to check out this PR's branch onto your machine.
Navigate in the command line to the packages/scripts folder (same location as the @wordpress/scripts package.json file)
Run npm link to create a link in node/npm
Go to your project and run npm link --save-dev @wordpress/scripts
This will install the scripts package you have locally as if it were a package in the project.
Note that your project package.json will now display a relative filepath for the scripts package:

This is essentially a symlink, which means if you were to make a change to the scripts, it would be instantly applied to wherever it's linked (great for quick dev changes).

npm link docs: https://docs.npmjs.com/cli/v10/commands/npm-link

gziolo · 2024-09-12T08:56:36Z

My testing results from local copy.

npm install locally doesn't apply changes ✅
npm run lint:css passes ✅
Stylistic modification get detected by npm run lint:css, example:

I also executed Prettier on all SCSS files which isn't configured correctly for WP coding conventions, this is the result in this branch:

The same exercise in the trunk:

Aside, it looks like all these violations caused by Prettier run and reported are fixable. I'm wondering if it's more reliable than what ESLint offers for JavaScript. Anyway, that's a separate story whether to add support for (S)CSS to wp-scripts format.

gziolo

It looks good to me. Excellent work bringing it to the finish line!

I would love to see more testing before landing, but we can also publish the dev version of the package to npm and see how fast it works for fo.

gziolo · 2024-09-12T09:10:19Z

packages/stylelint-config/index.js

@@ -1,7 +1,7 @@
 'use strict';

 module.exports = {
-	extends: 'stylelint-config-recommended',
+	extends: [ 'stylelint-config-recommended' ].map( require.resolve ),


I see it is used in other places with multiple items. It's fine.

mikeybinns · 2024-09-12T09:22:19Z

On the Prettier angle, that's exactly why I separated it out into separate configs. I personally use prettier on my personal projects, so I have no need for the formatting rules to be applied.

If you do go ahead and use prettier, you can switch the config in wp-scripts from scss-stylistic to scss and prevent the need to have something like eslint-config-prettier to disable all formatting rules. 🙂

Glad to see testing went well! 🥳

gziolo · 2024-09-12T11:31:18Z

I published all the updates included in this PR with the @next dist-tag to npm. You can test it in your projects with:

npm install @wordpress/stylelint-config@next --save-dev

See https://www.npmjs.com/package/@wordpress/stylelint-config/v/23.0.1-next.5368f64a9.0.

We have time until next Wednesday when the regular bi-weekly npm release happens to apply any necessary fixes if people report issues.

jacobcassidy · 2024-09-13T08:51:53Z

@gziolo The @next package doesn't include the scss-stylistic.js or stylistic.js files so when using the instructions to add:

{
	"extends": [ "@wordpress/stylelint-config/stylistic" ]
}

or

{
	"extends": [ "@wordpress/stylelint-config/scss-stylistic" ]
}

An error is thrown that those files don't exist.

gziolo · 2024-09-13T09:30:59Z

@gziolo The @next package doesn't include the scss-stylistic.js or stylistic.js files so when using the instructions to add:

Thank you for testing. Great catch, I can confirm that when checking the content of the package:

https://unpkg.com/browse/@wordpress/stylelint-config@23.0.1-next.5368f64a9.0/

Is anyone willing to contribute a change to the list of allowed files in the npm package?

gutenberg/packages/stylelint-config/package.json

Lines 26 to 32 in 632d8d6

    
           "files": [ 
        
           	"CHANGELOG.md", 
        
           	"LICENSE", 
        
           	"README.md", 
        
           	"index.js", 
        
           	"scss.js" 
        
           ],

mikeybinns · 2024-09-13T09:35:15Z

Is anyone willing to contribute...

I'm on it 👍

mikeybinns · 2024-09-13T09:42:44Z

Fix added here (didn't seem like I could add it to this PR anymore)
#65313

gziolo · 2024-09-13T09:43:57Z

Yes, new PR is the way to go. Reviewing 👍🏻

gziolo · 2024-09-13T10:55:30Z

New version is available on npm: https://www.npmjs.com/package/@wordpress/stylelint-config/v/23.0.1-next.1f6eadc42.0. I see both files when checking on unpkg.com:

jeffpaul · 2024-11-08T19:03:23Z

@vicobot-0815 mind sharing your WordPress.org username so I can ensure its included in the 6.7 credits listing?

mikeybinns added 6 commits August 27, 2024 00:39

update stylelint deps to latest and convert deprecated rules to @Styl…

62d4d2b

…istic

prevent extra rules being added

3cf6015

fix rule name change

734e8dd

fix scss overwrite bug

9e3a0e7

fix stylelint disables/enables

1b045f8

migrate stylistic styles to separate configs to make it easy to opt o…

412177e

…ut for consumers

mikeybinns requested review from ntwb, gziolo, nerrad, ajitbohra and ryanwelcher as code owners August 27, 2024 12:37

mikeybinns marked this pull request as draft August 27, 2024 13:26

mikeybinns added 2 commits August 27, 2024 23:21

fix: switch stylistic config dep for plugin

a7dbfba

use require.resolve for stylelint recommended config

db688a2

gziolo reviewed Aug 28, 2024

View reviewed changes

mikeybinns added 2 commits August 28, 2024 11:48

fix stylelint unit tests

6e6aebb

fix global mocks for node env

907f3a9

mikeybinns added 2 commits August 28, 2024 12:38

use correct configs for tests

1b90ab0

add type checking for configs

4835a90

mikeybinns added 2 commits August 28, 2024 13:16

convert tests to esm

ee076ce

remove unnecessary await

4003acb

fix stylelint tests

3efc42b

Merge branch 'trunk' into update-stylelint-dependency

754139f

gziolo approved these changes Sep 12, 2024

View reviewed changes

gziolo enabled auto-merge (squash) September 12, 2024 09:12

gziolo merged commit d649865 into WordPress:trunk Sep 12, 2024
60 checks passed

github-actions bot added this to the Gutenberg 19.3 milestone Sep 12, 2024

mikeybinns deleted the update-stylelint-dependency branch September 12, 2024 09:22

mikeybinns restored the update-stylelint-dependency branch September 13, 2024 09:34

mikeybinns mentioned this pull request Sep 13, 2024

Styleling config: Fix stylelint config missing files for npm #65313

Merged

gziolo mentioned this pull request Sep 19, 2024

[eslint-plugin] Update eslint to v9 and typescript-eslint to v8 #64782

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scripts: Update stylelint dependency and the default config #64828

Scripts: Update stylelint dependency and the default config #64828

mikeybinns commented Aug 27, 2024 •

edited

Loading

github-actions bot commented Aug 27, 2024 •

edited

Loading

mikeybinns commented Aug 27, 2024

mikeybinns commented Aug 27, 2024

gziolo commented Aug 28, 2024 •

edited

Loading

gziolo Aug 28, 2024

mikeybinns Aug 28, 2024

gziolo Sep 12, 2024

gziolo commented Aug 28, 2024

mikeybinns commented Aug 28, 2024

mikeybinns commented Aug 28, 2024 •

edited

Loading

mikeybinns commented Aug 29, 2024

mikeybinns commented Sep 6, 2024 •

edited

Loading

gziolo commented Sep 12, 2024 •

edited

Loading

gziolo left a comment

gziolo Sep 12, 2024

mikeybinns commented Sep 12, 2024

gziolo commented Sep 12, 2024

jacobcassidy commented Sep 13, 2024

gziolo commented Sep 13, 2024

mikeybinns commented Sep 13, 2024

mikeybinns commented Sep 13, 2024

gziolo commented Sep 13, 2024

gziolo commented Sep 13, 2024

jeffpaul commented Nov 8, 2024

Scripts: Update stylelint dependency and the default config #64828

Scripts: Update stylelint dependency and the default config #64828

Conversation

mikeybinns commented Aug 27, 2024 • edited Loading

What?

Why?

How?

Testing Instructions

github-actions bot commented Aug 27, 2024 • edited Loading

Unlinked Accounts

mikeybinns commented Aug 27, 2024

mikeybinns commented Aug 27, 2024

gziolo commented Aug 28, 2024 • edited Loading

gziolo Aug 28, 2024

Choose a reason for hiding this comment

mikeybinns Aug 28, 2024

Choose a reason for hiding this comment

gziolo Sep 12, 2024

Choose a reason for hiding this comment

gziolo commented Aug 28, 2024

mikeybinns commented Aug 28, 2024

mikeybinns commented Aug 28, 2024 • edited Loading

mikeybinns commented Aug 29, 2024

mikeybinns commented Sep 6, 2024 • edited Loading

gziolo commented Sep 12, 2024 • edited Loading

gziolo left a comment

Choose a reason for hiding this comment

gziolo Sep 12, 2024

Choose a reason for hiding this comment

mikeybinns commented Sep 12, 2024

gziolo commented Sep 12, 2024

jacobcassidy commented Sep 13, 2024

gziolo commented Sep 13, 2024

mikeybinns commented Sep 13, 2024

mikeybinns commented Sep 13, 2024

gziolo commented Sep 13, 2024

gziolo commented Sep 13, 2024

jeffpaul commented Nov 8, 2024

mikeybinns commented Aug 27, 2024 •

edited

Loading

github-actions bot commented Aug 27, 2024 •

edited

Loading

gziolo commented Aug 28, 2024 •

edited

Loading

mikeybinns commented Aug 28, 2024 •

edited

Loading

mikeybinns commented Sep 6, 2024 •

edited

Loading

gziolo commented Sep 12, 2024 •

edited

Loading