Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades ESLint from v8 to v9, migrates to a flat config format, and cleans up legacy lint files and plugins.
- Migrated from
.eslintrc.jsontoeslint.config.mjsusing@eslint/eslintrcand@eslint/js - Removed legacy lint configs (
tsconfig.lint.json,.eslintrc.json,.eslintignore) and dropped unused plugins - Updated
package.jsonscripts and dependencies to align with ESLint v9 flat config
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.lint.json | Removed legacy TypeScript lint config |
| src/deserialize.ts | Dropped an ESLint disable comment on unsafe assignment |
| package.json | Updated lint scripts and bumped ESLint-related deps |
| eslint.config.mjs | Added new flat ESLint config, plugin imports, and rules |
| build.mjs | Adjusted sourcemap condition in esbuild config |
| .eslintrc.json | Deleted old ESLint JSON config |
| .eslintignore | Deleted legacy ignore file |
Comments suppressed due to low confidence (3)
src/deserialize.ts:5
- The ESLint disable comment for
@typescript-eslint/no-unsafe-assignmentwas removed, which may now cause lint failures. Either re-introduce a targeted rule override or refine the typing ofvalueto satisfy the rule.
value[l] = value[k];
eslint.config.mjs:8
- The import path for the TypeScript ESLint plugin is incorrect. It should import from "@typescript-eslint/eslint-plugin" to access
tseslint.configandtseslint.configs.
import tseslint from "typescript-eslint";
eslint.config.mjs:16
- You configure only the
no-async-foreachplugin via FlatCompat, but you still referenceimport/*rules below. Add...compat.plugins("import")(or similar) so import rules load correctly.
...compat.plugins("no-async-foreach"),
build.mjs
Outdated
| platform: "node", | ||
| format: "cjs", | ||
| sourcemap: !!process.env.CODEQL_VARIANT_ANALYSIS_ACTION_GENERATE_SOURCEMAPS | ||
| sourcemap: process.env.CODEQL_VARIANT_ANALYSIS_ACTION_GENERATE_SOURCEMAPS |
There was a problem hiding this comment.
Using the raw environment variable string here will always be truthy when set. Wrap it in a boolean conversion (!!process.env...) to ensure correct conditional behavior.
| sourcemap: process.env.CODEQL_VARIANT_ANALYSIS_ACTION_GENERATE_SOURCEMAPS | |
| sourcemap: !!process.env.CODEQL_VARIANT_ANALYSIS_ACTION_GENERATE_SOURCEMAPS |
There was a problem hiding this comment.
Hmm, I think eslint --fix made this change, but I'm not sure why. I'll revert it back to using !!
koesie10
left a comment
koesie10
left a comment
There was a problem hiding this comment.
LGTM, and the config looks like it matches.
eslint.config.mjs
Outdated
| { | ||
| extends: [eslint.configs.recommended, tseslint.configs.recommended], | ||
| }, |
There was a problem hiding this comment.
Why do we need to use extends here, but not for any of the other configs?
eslint.config.mjs
Outdated
|
|
||
| export default tseslint.config( | ||
| globalIgnores(["dist/", "node_modules/", "script/", "jest.config.ts", "eslint.config.mjs", "build.mjs"]), | ||
| ...compat.plugins("no-async-foreach"), |
There was a problem hiding this comment.
Do we still need the no-async-foreach plugin? It seems like this is caught by at least three different rules now:
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This PR upgrades ESLint to version 9.x.x. The conversion was quite involved, and there is a conversion tool but it found it was quite confusing and didn't work too well so I had to do it manually. It's been a bit of a struggle and an adventure but this is what I've ended up with. As far as I can tell it's all working 🤞🏼
This PR supersedes #1212 and #1185.
There are a couple of small changes I made during the migration:
eslint-plugin-importbecause it's already included ineslint-plugin-github(and was causing errors trying to redeclare it in the config file). As far as I can tell this hasn't changed any behaviour.eslint-plugin-filenamesbecause it's quite old and even usingcompatit wasn't working. I just wasn't sure it was important enough to continue messing around with so I removed it, but please say if you think that was a mistake.