Bump ESLint and related dependencies #254
Conversation
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is manifest confusion?This package has inconsistent metadata. This could be malicious or caused by an error when publishing the package. Packages with inconsistent metadata may be corrupted or malicious. Take 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
|
There was a problem hiding this comment.
I know that the createConfig function was already released and published. However, I can't help but wonder whether it will, in the long run, make our ESLint configurations unnecessarily difficult to understand and debug. ESLint already provides recommendations for combining configuration objects (and exporting configs so they can be properly combined), and these are what plugins like eslint-plugin-import-x, eslint-plugin-jsdoc, and eslint-plugin-prettier follow in switching to the flat config format.
Upon switching to ESLint 9, I would expect the config file to look like the following:
import baseConfigs from '@metamask/eslint-config';
import typescriptConfigs from '@metamask/eslint-config-typescript';
import nodejsConfigs from '@metamask/eslint-config-nodejs';
import jestConfigs from '@metamask/eslint-config-jest';
export default [
...baseConfigs,
{
ignores: ['dist/', 'docs/', '.yarn/'],
},
{
languageOptions: {
sourceType: 'module'
},
settings: {
'import-x/extensions': ['.js', '.mjs'],
},
},
...typescriptConfigs.map((config) => ({
files: ['**/*.ts'],
languageOptions: {
parserOptions: {
tsconfigRootDir: import.meta.dirname,
project: ['./tsconfig.json'],
},
},
}}),
...nodeJsConfigs.map((config) => ({
...config,
files: ['**/*.js', '**/*.cjs'],
languageOptions: {
sourceType: 'script',
},
})),
...nodeJsConfigs.map((config) => ({
...config,
files: ['**/*.test.ts', '**/*.test.js'],
languageOptions: {
sourceType: 'script',
},
})),
...jestConfigs.map((config) => ({
...config,
files: ['**/*.test.ts', '**/*.test.js'],
})),
];While using map is a bit more verbose, I think it's very clear that we are creating objects to add to the flat array that we are ultimately exporting. I am not sure that is so clear with the createConfig utility.
What are your thoughts on this? I believe there are also further improvements we can make to the ESLint packages so that we can streamline this file, but I will leave that for a future comment.
There was a problem hiding this comment.
I considered this, and after talking about it with @rekmarks, we decided to create a simple util function that handles it for you. I don't think it changes so much in understandability of the config, and because it's less verbose, I feel like it's easier to read.
The function is based on typescript-eslint's config function, so it's not like we're the only ones doing this.
Upon switching to ESLint 9, I would expect the config file to look like the following:
[...]
This gets very verbose if you actually want to override some rules or other settings. For example:
...typescript.map((config) => {
...config,
rules: {
...config.rules,
'some-rule': 'off',
},
languageOptions: {
...config.languageOptions,
parserOptions: {
...config.languageOptions.parserOptions,
ecmaVersion: 2023,
}
}
});Of course we could use some deep merge function, but that's essentially what createConfig does, but easier.
I can't help but wonder whether it will, in the long run, make our ESLint configurations unnecessarily difficult to understand and debug.
I don't think this will be any more of a problem with createConfig compared to map and spreading the config. Either way it's quite easy to log the object to see what's going on.
There was a problem hiding this comment.
I find ESLint's recommendations—i.e. the map() syntax—to be completely unreadable, and much prefer the utility function. In fact, during the migration of our monorepo to ESLint 9, not using the createConfig() function was more difficult and bug-prone, as opposed to the reverse.
There was a problem hiding this comment.
Thanks for the replies. That helps me understand the reasoning better.
I do wish it were more obvious that an object with extends actually represents multiple objects; that seems to diverge from the central ideas behind flat config. But, if it succeeds in helping us write the config files we want to write, then perhaps it doesn't matter.
ESLint 8 and the legacy config format are EOL. In this PR I've updated ESLint to v9, and bumped all related dependencies (ESLint configs, Prettier,
auto-changelog), and rewritten the config to use the flat config format.