Add ESLint integration

[westonruter]

Update: The JSCS project has merged into ESLint. So now an ESLint config is needed to be developed for core as opposed to a JSCS one.

---

The ​JSCS project has ​added a ​wordpress preset. This could be useful to be included in Core. All it needs is a .jscsrc file located in the root which contains:

{
        "preset": "wordpress",
        "excludeFiles": [
                "**/vendor/**",
                "**.min.js",
                "**/node_modules/**"
        ]
}

Related:
#30153 (PHP_CodeSniffer)
#25187 (JSHnt)
#28543 (Allow stricter JSHint checks for core files)

[netweb]

Related: #28236

I have some large concerns in adding JSCS in any form to WordPress Core.

I looked into adding JSCS to WordPress Core to fix #28236 back in January. To get familiar with JSCS I looked at jQuery's implementation and upon looking found that jQuery needed some JSCS updates. I created two pull requests for jQuery, one was merged and the second was not (The JSCS maintainers maintain jQuery's implementation of JSCS.)

I walked away from the second and from JSCS in frustration of ideological differences of what I believe should've been merged vs what the JSCS maintainers believe should've been merged.

For reference these are the two jQuery issues:

• ​https://github.com/jquery/jquery/pull/2029
• ​https://github.com/jquery/jquery/pull/2030

As I noted in #29792

This is becoming an important issue for us, our tools need to be both functional and maintained and is one part of the reasoning for the change in #31332

I'll add to this by saying that I think any external libraries we use should also match WordPress Core development philosophies and based on my recent JSCS experience via jQuery at this time I don't believe JSCS align with WordPress'.

[westonruter]

Just got this ​update from @hzoo:

If you guys do add it to wordpress core (or even just run it on the codebase) - there will probably be a lot of changes needed to the preset (added and removed). Like ​#1187.

So you can create an issue/PR about it so we can get it in.

And if there's differences in rules because of jquery changes we could move the original jquery rules into wordpress so it doesn't depend on another preset (if needed).

Maybe it would be interesting to not have to depend on jscs itself for the preset definitions (but that's probably a different thing altogether and maybe already discussed?).

Disclaimer: I'm new to JSCS.

[iseulde]

This is now merged with ESLint, see ​http://jscs.info.

[tfrommen]

I just came across this ticket. Not sure how things are going here, but at Inpsyde, we are using ESLint for a longer time already.

Quite a while ago, I created a ​Modern JavaScript Style Guide that is heavily influenced by the ​WordPress JavaScript Coding Standards as well as the ​Airbnb JavaScript Style Guide, which also includes ES6-specific content. Based on that style guide, I then created a complete ​shareable ESLint config, following WordPress coding standards, and at the same time incorporating modern JavaScript, such as ES6 features. For more background, please refer to an ​ESLint (for WordPress) introduction post on my personal blog.

I just wanted to leave this here for consideration and/or discussion. The config could at least serve as a starting point, if not really the base of an official WordPress ESLint config.

[westonruter]

There also an .eslintrc in wp-dev-lib for reference: ​https://github.com/xwp/wp-dev-lib/blob/master/.eslintrc

[jorbin]

I think it's time for us to switch to ESLint. It will make the code look more like it's all written by a single person, improving readability, and prepare us for writing more ES2015+ code.

First, we need to solidify an eslint config, then we need to ​do a project like we did with jshint, and tackle each javascript file one at a time. Then, we can remove jshint and replace all of the grunt calls for jshint with eslint.

[netweb]

Here's a first pass for a new Grunt task grunt eslint in 31823.diff​:

• It uses the ESLint "shared config" ​https://github.com/WordPress-Coding-Standards/eslint-config-wordpress

• It uses version 1.0.0 of the "shared config" which is designed as a "drop in" replacement for WordPress' .jshintrc

• I designed a roadmap for future releases of eslint-config-wordpress, this would allow users of the shared config to "pin" to a version of the config that would be compatible with their codebase until such time they've updated their codebase and able to move to a newer version of the config

• Version 1.0.0 - Parity with WordPress' current .jshintrc

• Version 1.1.0 - Parity with WordPress' current .jshintrc with the extra now deprecated JSHint onevar and trailing options

• Version 2.0.0 - Parity with the JSCS's WordPress config

• Version 3.0.0 - Utilize and leverage ESLints rules to add sane defaults and further enhance WordPress' existing coding standards

• Full roadmap details: ​https://github.com/WordPress-Coding-Standards/eslint-config-wordpress/issues/8

The Grunt task currently errors on a handful of issues in Gruntfile.js and a Twenty Fourteen JS file, to force the Grunt task to run all the subtasks to see what ESLint errors are shown grunt eslint --force should do the trick.

[adamsilverstein]

31823.2.diff​ fixes the linting errors you noted in gruntfile and twenty-fourteen

[netweb]

31823.4.diff​ utilises ESLint's --fix option based on the updated eslint-config-wordpress version `2.0.0`

Command: eslint Gruntfile.js --config node_modules/eslint-config-wordpress/index.js --fix

[netweb]

31823.5.diff​ is a refresh of the previous two patches with a single change, rather than grunt eslint:jshint for "core" files the task is now named grunt eslint:core

[westonruter]

Let's wait to fix until 4.9 is shipped. It should be an early task. It should be done the same time as #41057.

For the initial commit, let's just get an .eslintrc into trunk to start.

[netweb]

Replying to westonruter:

Let's wait to fix until 4.9 is shipped. It should be an early task. It should be done the same time as #41057.

For the initial commit, let's just get an .eslintrc into trunk to start.

Punting, the current state of the JS Coding Standards is in flux for another couple of weeks, once that stabilises I'll upload a patch and to make available a new Grunt task for ESLint though keeping it excluded from the build process until a fix is ready to patch all existing JS files.

[netweb]

In 31823.6.diff​:

• First pass at a quick drop-in replacement for JSHint to address the licensing issue in #42850
• Uses eslint-config-wordpress version 1.0.0 which was designed as a "drop in" replacement for the current JSHint .jshintrc file

There's more to do here, remove JSHint, for the moment though this allows JSHint and ESLint to be run side-by-side

[davidbaumwald]

This ticket hasn't seen any movement in a while, so it's being moved to Future Release. If it's decided it can land in a specific release, the milestone can be updated during that cycle.

As per ​https://make.wordpress.org/core/2022/03/23/migrating-wordpress-e2e-tests-to-playwright/, this migrates all Puppeteer usage in core to Playwright.

To-do

• [ ] ~Maybe set up ESLint/Prettier for formatting...? Odd that core doesn't have this yet 🤷 ~ see https://core.trac.wordpress.org/ticket/31823
• [ ] Publish update on make/core after merging
  • Mention the change from tests/visual-regression/specs/__image_snapshots__ to tests/visual-regression/specs/__snapshots__ for visual tests snapshots

Trac ticket: https://core.trac.wordpress.org/ticket/59517

Committed in https://core.trac.wordpress.org/changeset/56926 🎉

Thanks y'all!

[swissspidy]

Prettier is now configured since #60316 / r57330

[youknowriad]

The latest patch is still using the eslint-config-wordpress package which I believe has been deprecated. I'm guessing we should update the patch to use @wordpress/eslint-plugin instead.

[swissspidy]

In 57702:

Build/Test Tools: Update JSHint config to remove deprecated options.

Removes deprecated options that no longer have any effect, and updates the targeted ES version in line with WordPress’ browser support.

This change mostly allows new code to properly use trailing commas, as added by the Prettier formatter.

Future efforts should rather go towards adopting ESLint for code formatting, see #31823.

Props netweb.
Fixes #28236.

Here be dragons...

Trac ticket: https://core.trac.wordpress.org/ticket/31823

TODO: Enable 'plugin:@wordpress/eslint-plugin/test-playwright', for Playwright tests

TODO: Add ​https://github.com/ylemkimon/eslint-plugin-actions if it still works

Maybe ​https://github.com/BenoitZugmeyer/eslint-plugin-html too

It looks like there's currently one warning. Since there's only one, do we want to elevate warnings to fail to prevent more warnings from being introduced?

Oh I missed that one 😄 Yes indeed, such rules should all be elevated to be errors & fixed or ignored with an explanation. I'll get to it.

@desrosj Any idea why the Windows builds might be failing with Warning: The build\wp-includes\js\dist\components.js file must not contain a sourceMappingURL. Use --force to continue.?

I recall some recent discussions about sourceMappingURL around the 6.5.2 release, but this PR shouldn't be touching those packages, so not sure why that fails 🤷

@swissspidy I'm not sure why yet, but it looks like the change that added glob to devDependencies is to blame.

I created #6486 to step through the changes to package(-lock).json one at a time, and the ​build started failing when I pinned glob.

When I ​reverted that change and left all the other ones, it passed again. Looking at the dependency tree for npm list glob before adding that to our package.json file, there were 15 occurrences with a mix of version 7.1.6 and 7.2.3. The version pinned here is 10.3.12.

I think we could take a few approaches here:

• remove this from our package file and let npm manage it as a transitive dependency.
• figure out what the breaking change is.
• pin version 7.2.3 of glob for now and explore updating it separately. I tested this, and even though it updates the package for the packages expecting 7.1.6 it ​seems to work as expected.

Looks like it's something in the 8.x to 9.x update.

​Updating to 8.x passes while ​9.x fails.

Thank you so much for testing! 🚀

Pinning to 7.x or 8.x makes sense.