-
Notifications
You must be signed in to change notification settings - Fork 16
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
BREAKING: Bump ESLint to ^9.11.1
, bump related ESLint dependencies, and rewrite configs to use flat configs
#370
Conversation
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
^9.11.1
, bump related ESLint dependencies, and rewrite configs to use flat configs^9.11.1
, bump related ESLint dependencies, and rewrite configs to use flat configs
a3a6e99
to
cfd2c99
Compare
4ba8126
to
69c38f9
Compare
@SocketSecurity ignore npm/agentkeepalive@4.5.0 Network access expected. @SocketSecurity ignore npm/@npmcli/fs@2.1.2 New author is ok. @SocketSecurity ignore npm/gauge@4.0.4 Deprecated is ok. @SocketSecurity ignore npm/eslint-plugin-jest@28.8.3 Seems like a false positive. |
scripts/validate-configs.mjs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script doesn't have the same functionality as the previous script. It's a bit hard to follow what exactly it was doing, so for the time being, this script simply checks and writes snapshots.
@@ -6,38 +6,50 @@ | |||
"type": "git", | |||
"url": "https://github.com/MetaMask/eslint-config.git" | |||
}, | |||
"type": "module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this safe because consumers always use this package as a devDep? Are we sure this doesn't affect downstream CJS packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the eslint v9 migrate guide, looks like we only need to worry about CJS configurations importing from our repo, and that shouldn't be a problem for long since eslint is now set up to use ESM by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consumers can add it and use it even if they use type: commonjs
, as long as the config is a .mjs
file. I figured we might as well update all places where we consume the libraries since we have to rewrite the majority of the configs anyway.
@metamaskbot publish-preview |
@SocketSecurity ignore npm/node-gyp@10.2.0 New author is ok. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Confirmed that the updated config packages can be installed and are functional downstream. IMO any further fixes that may or may not be needed can be handled in separate PRs.
ESLint 8 will be end-of-life from 2024-10-05. ESLint 8 introduced a new config format, which is now the default in ESLint 9, and only opt-out through an environment flag. We could enable that flag and keep the current configs, but it's better to just update the configs now and be done with it.
I've updated all configs to use the new format. The most notable changes are:
Closes #362.
Breaking changes
All configs
^9.11.0
, and all configs were updated to use the flat config format.@metamask/eslint-config-typescript
@typescript-eslint/parser
and@typescript-eslint/eslint-plugin
are replaced withtypescript-eslint@^8.6.0
, which is now a peer dependency.languageOptions.parserOptions.tsconfigRootDir
must be set to the current directory, e.g., usingimport.meta.dirname
.