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

@wordpress/scripts lint-js not compatible with newer versions of ESLint (ESLint 9+) #55499

Closed
ethanscorey opened this issue Oct 20, 2023 · 7 comments
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Tool] ESLint plugin /packages/eslint-plugin [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.

Comments

@ethanscorey
Copy link

Description

The lint-js script in the @wordpress/scripts package does not work properly with the new ESLint configuration file format (I tested on 8.51.0, but it likely affects older versions as well). It appears that lint-js is passing obsolete CLI arguments to ESLint, causing the errors.

For example, if you run it without specifying the --config parameter, you get Invalid option '--eslintrc' - perhaps you meant '--ignore'?

If you try to resolve the error by adding the --config param, you get this error instead: Invalid option '--ext' - perhaps you meant '-c'?

Step-by-step reproduction instructions

  1. Create a new directory.
  2. Run npm init and `npm install '@wordpress/scripts' --save-dev
  3. Create a eslint.config.js file with any valid config options.
  4. Run npx wp-scripts lint-js
  5. Run npx wp-scripts lint-js --config eslint.config.js

Screenshots, screen recording, code snippet

No response

Environment info

Device: Arch Linux with kernel version Linux 6.5.3-zen1-1-zen

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@gziolo gziolo added the [Tool] ESLint plugin /packages/eslint-plugin label Oct 20, 2023
@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Oct 22, 2023
@gziolo gziolo added Needs Dev Ready for, and needs developer efforts Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Nov 7, 2023
@gziolo
Copy link
Member

gziolo commented Nov 7, 2023

It looks like the code doesn't detect the new config format introduced recently, as I don't see eslint.config.js listed:

// See: https://eslint.org/docs/user-guide/configuring#using-configuration-files-1.
const hasLintConfig =
hasArgInCLI( '-c' ) ||
hasArgInCLI( '--config' ) ||
hasProjectFile( '.eslintrc.js' ) ||
hasProjectFile( '.eslintrc.json' ) ||
hasProjectFile( '.eslintrc.yaml' ) ||
hasProjectFile( '.eslintrc.yml' ) ||
hasProjectFile( 'eslintrc.config.js' ) ||
hasProjectFile( '.eslintrc' ) ||
hasPackageProp( 'eslintConfig' );

Another question that might be related here is whether we want to start using the new config format introduced in v8.21.0 as the default config:

// When a configuration is not provided by the project, use from the default
// provided with the scripts module. Instruct ESLint to avoid discovering via
// the `--no-eslintrc` flag, as otherwise it will still merge with inherited.
const defaultConfigArgs = ! hasLintConfig
? [ '--no-eslintrc', '--config', fromConfigRoot( '.eslintrc.js' ) ]
: [];

@Utsav-Ladani
Copy link

Hi @gziolo

I tested this bug, and here are my findings:

  • The new eslint supports the eslint.config.js file called flat config.
  • In the environment variable, we can deactivate/activate it using ESLINT_USE_FLAT_CONFIG.
  • The --config is still a valid parameter (not working here).
  • But the ext is not a valid parameter. Instead, we can specify the extension names in the files property in the eslint.config.js file as an array.
  • Same for the --ignore-path parameter. We can use the ignores property for that.

I have two solutions in my mind:

  1. Dual Eslint Version Support: Add support for both eslint versions by adding the version check condition
  2. Single Version Focus with Upcoming Release: Release the new package version with only the latest eslint support, as the flat config will be the default option in the upcoming eslint v9.0.0.

Please let me know, as I am a newbie to this package management system and may need a different approach.

@swissspidy swissspidy added the [Tool] WP Scripts /packages/scripts label Apr 24, 2024
@swissspidy swissspidy changed the title @wordpress/scripts lint-js not compatible with newer versions of eslint @wordpress/scripts lint-js not compatible with newer versions of ESLint (ESLint 9+) Apr 24, 2024
@swissspidy
Copy link
Member

Forcing everyone to use ESLint 9, even if done in a new major release, is going to be very painful. I suggest supporting both versions as long as possible, maybe with versions checks or by leveraging peerDependencies or something.

@colorful-tones
Copy link
Member

I think I'm encountering issues that are related to this, but not entirely certain. I've been trying to get ESLint operational in VS Code. I have ESLint extension activated, but I'm seeing issues with Gutenberg's configuration:

[Info  - 12:08:33 PM] ESLint server is starting.
[Info  - 12:08:33 PM] ESLint server running in node v20.11.0
[Info  - 12:08:33 PM] ESLint server is running.
[Info  - 12:08:34 PM] ESLint library loaded from: /Users/user/Sites/gutenberg-trunk/app/public/wp-content/plugins/gutenberg-trunk/node_modules/eslint/lib/api.js
[Error - 12:08:34 PM] An unexpected error occurred:
[Error - 12:08:34 PM] Error: ESLint configuration in plugins/gutenberg-trunk/.eslintrc.js is invalid:
	- Property "overrides[9].files" is the wrong type (expected string but got `[]`).
	- "overrides[9].files" should NOT have fewer than 1 items. Value: [].
	- "overrides[9].files" should match exactly one schema in oneOf. Value: [].

There is more, but I'm truncating it there

@swissspidy
Copy link
Member

@colorful-tones I think that's unrelated.

The ninth overrides.files entry is this:

files: typedFiles,

For some reason typedFiles is empty in your project.

gutenberg/.eslintrc.js

Lines 34 to 37 in 9cc96a7

// All files from packages that have types provided with TypeScript.
const typedFiles = glob( 'packages/*/package.json' )
.filter( ( fileName ) => require( join( __dirname, fileName ) ).types )
.map( ( fileName ) => fileName.replace( 'package.json', '**/*.js' ) );

@colorful-tones
Copy link
Member

Great spot there @swissspidy!

I would think running npm run clean:package-types

"clean:package-types": "tsc --build --clean",

and npm run build:package-types would solve this issue of not having any tsc files?

"build:package-types": "node ./bin/packages/validate-typescript-version.js && tsc --build && node ./bin/packages/check-build-type-declaration-files.js",

But it does not and I see TypeScript issues in console when attempting to lint, e.g. npm run lint:js -c packages/editor/src/autosave-monitor/index.js

=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=4.3.5 <5.2.0

YOUR TYPESCRIPT VERSION: 5.4.5

Please only submit bug reports when using the officially supported version.

=============

However, this is all likely not related to this main issue. It does seem like ESLint could use some attention in Gutenberg though, as there are quite a few issues that are overlapping (or seems so) with mine. #54305, and the other 231 other results.

Anyways, not related and please disregard. Thanks!

@swissspidy
Copy link
Member

Closing in favor of #64782 which has an active PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Tool] ESLint plugin /packages/eslint-plugin [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants