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

Switch to ESLint v9 and flat config #3558

Merged
merged 7 commits into from
Sep 25, 2024

Conversation

KristjanESPERANTO
Copy link
Contributor

Since PR #3551 was not yet complete, I made my own attempt.

  1. Update to ESLint v9.
  2. Replace deprecated .eslintrc.json and .eslintignore by flat config eslint.config.mjs.
  3. Adapt check_config.js to use flat config.
  4. Since eslint-plugin-import still doesn't support ESLint v9 I removed it. We can add it back when it does support v9.
  5. Run tests npm run check:js and npm run config:check.
  6. In order not to overload this PR, I have not yet activated more additional rules - there are some useful ones in the new plugin @eslint/js.

@bugsounet, please don't take it as an offence that I have created a competing PR. The migration to ESLint v9 has been burning under my nails for some time.

@bugsounet
Copy link
Contributor

Don't worry, I just wanted to start coding the tests with eslint v9.
I know that you are more skilled than me on this ;)

@khassel khassel requested a review from rejas September 25, 2024 17:09
@rejas rejas merged commit d318768 into MagicMirrorOrg:develop Sep 25, 2024
8 checks passed
@khassel
Copy link
Collaborator

khassel commented Oct 5, 2024

@KristjanESPERANTO

to be honest I have no clue about eslint but with this PR we have

	"dependencies": {
		"eslint": "^9.11.1",
	},
	"devDependencies": {
		"@eslint/js": "^9.11.1",
	},

in package.json, the versions are the same and both npm packages refer to the same github repo.

Is this correct or would be the one under dependencies sufficient?

@KristjanESPERANTO
Copy link
Contributor Author

eslint is the real ESLint which we need in dependencies because we use it in the config check.

@eslint/js is a plugin for ESLint that we use for linting - so devDependecies is the right place for it.

So everything is fine in my eyes.

@khassel
Copy link
Collaborator

khassel commented Oct 5, 2024

Thanks for the explanation, I checked for dependency updates and saw the same update for both ...

@KristjanESPERANTO
Copy link
Contributor Author

The developers seem to synchronize the version number of both. So we will be seeing this even more often in the future.

I understand the doubt 🙂 They should have found a better name for the plugin.

@khassel
Copy link
Collaborator

khassel commented Oct 5, 2024

hm, I tested now without the @eslint/js (see this commit) and all tests were fine (?)

@KristjanESPERANTO
Copy link
Contributor Author

Yes, it works because the plugin is a dependency of ESLint.

https://github.com/eslint/eslint/blob/main/package.json#L104

Isn't it recommended to put direct dependencies into the package.json? We import the plugin in the ESLint config.

khassel pushed a commit that referenced this pull request Oct 13, 2024
eslint-plugin-import was missing since the switch to
[v9](#3558). They
finally
[support](import-js/eslint-plugin-import#2996)
it so we can re-add it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants