Update ESLint to v9+ #31
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Taking a friendly look at your code with a bit of AI assistance. These suggestions are possibilities, not promises, so use what fits your style. You’re in control—AI is here to lend a hand, not call the shots.
Model: o4-mini | Prompt Tokens: 6642 | Completion Tokens: 2807
pr-review/eslint.config.js
Outdated
| import sonarjs from "eslint-plugin-sonarjs"; | ||
| import eslintPluginUnicorn from "eslint-plugin-unicorn"; | ||
| import { defineConfig, globalIgnores } from "eslint/config"; | ||
| import tseslint from "typescript-eslint"; |
There was a problem hiding this comment.
It looks like you’re importing TypeScript ESLint under the wrong package name. The correct scoped package is @typescript-eslint/eslint-plugin, and you should import from its flat-helpers entrypoint for a flat config:
-import tseslint from "typescript-eslint";
+import tseslint from "@typescript-eslint/eslint-plugin/flat";| export default defineConfig([ | ||
| globalIgnores(["dist/*"]), | ||
| { | ||
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | ||
| plugins: { js }, | ||
| extends: ["js/recommended"], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| projectService: { allowDefaultProject: ["eslint.config.js"] }, | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| }, | ||
| }, | ||
| importPlugin.flatConfigs.recommended, | ||
| importPlugin.flatConfigs.typescript, | ||
| tseslint.configs.recommended, | ||
| tseslint.configs.recommendedTypeChecked, | ||
| tseslint.configs.stylisticTypeChecked, | ||
| sonarjs.configs.recommended, | ||
| eslintPluginUnicorn.configs.recommended, |
There was a problem hiding this comment.
Your ESLint configuration for both pr-review and pr-summary is almost identical. To avoid duplication and ensure consistency, consider extracting the shared parts into a common module (e.g. eslint.common.config.js) and then spreading it:
// eslint.common.config.js
import js from "@eslint/js";
import importPlugin from "eslint-plugin-import";
import sonarjs from "eslint-plugin-sonarjs";
import unicorn from "eslint-plugin-unicorn";
import tsEslint from "@typescript-eslint/eslint-plugin/flat";
import { defineConfig, globalIgnores } from "eslint/config";
export const shared = [
globalIgnores(["dist/*"]),
{
files: ["**/*.{js,mjs,cjs,ts,mts,cts}"],
plugins: { js },
extends: ["js/recommended"],
languageOptions: {
parserOptions: {
projectService: { allowDefaultProject: ["eslint.config.js"] },
tsconfigRootDir: import.meta.dirname,
},
},
},
importPlugin.flatConfigs.recommended,
importPlugin.flatConfigs.typescript,
tsEslint.configs.recommended,
tsEslint.configs.recommendedTypeChecked,
tsEslint.configs.stylisticTypeChecked,
sonarjs.configs.recommended,
unicorn.configs.recommended,
];
defineConfig([...shared, { rules: { /* overrides */ } }]);Then in each project’s eslint.config.js, simply import and extend this shared array.
| // eslint-disable-next-line @typescript-eslint/no-for-in-array | ||
| for (const key in actionYaml.inputs) { | ||
| const envKey = `INPUT_${key.toUpperCase()}` |
There was a problem hiding this comment.
You’re iterating over an array of inputs using for..in, which is meant for object keys. Switching to for..of with Object.entries improves clarity and avoids lint ignores:
-for (const key in actionYaml.inputs) {
- const envKey = `INPUT_${key.toUpperCase()}`;
- const envValue = actionYaml.inputs[key]?.default;
- // ...
-}
+for (const [key, { default: envValue }] of Object.entries(actionYaml.inputs)) {
+ const envKey = `INPUT_${key.toUpperCase()}`;
+ // use envValue directly here
+ // ...
+}| let lineNo = 0 | ||
|
|
||
| const fileStatus = file.deleted ? "deleted" : file.new ? "added" : file.from !== file.to ? "renamed" : "modified" // eslint-disable-line sonarjs/no-nested-conditional | ||
| const fileStatus = file.deleted ? "deleted" : file.new ? "added" : file.from !== file.to ? "renamed" : "modified" // eslint-disable-line sonarjs/no-nested-conditional, unicorn/no-nested-ternary |
There was a problem hiding this comment.
The nested ternary for fileStatus is hard to read and suppressed via unicorn/no-nested-ternary. Refactor into an if/else chain for clarity:
-let fileStatus = file.deleted ? "deleted" : file.new ? "added" : file.from !== file.to ? "renamed" : "modified";
+let fileStatus: string;
+if (file.deleted) {
+ fileStatus = "deleted";
+} else if (file.new) {
+ fileStatus = "added";
+} else if (file.from !== file.to) {
+ fileStatus = "renamed";
+} else {
+ fileStatus = "modified";
+}| core.info(`Reading context file ${file.path}`) | ||
| // eslint-disable-next-line no-await-in-loop | ||
| const { data: blob } = await octokit.rest.git.getBlob({ ...repoRef, file_sha: file.sha, mediaType: { format: "raw" } }) | ||
| const result = [`Context file ${file.path}:`, "```", blob as unknown as string, "```", ""] |
There was a problem hiding this comment.
await inside a for..of loop will run sequentially, which can slow down reading blobs in series. If order isn’t critical, consider collecting promises and await Promise.all to parallelize:
// Original sequential loop
for (const file of tree) {
if (shouldFetch(file)) {
const { data: blob } = await octokit.rest.git.getBlob(
{ ...repoRef, file_sha: file.sha, mediaType: { format: "raw" } }
);
// ...
}
}
// Parallel version
const fetches = tree
.filter(shouldFetch)
.map(async file => {
const { data: blob } = await octokit.rest.git.getBlob(
{ ...repoRef, file_sha: file.sha, mediaType: { format: "raw" } }
);
return { path: file.path, blob };
});
const results = await Promise.all(fetches);
results.forEach(({path, blob}) => {
core.info(`Context file ${path}`);
// ...
});| { | ||
| "$schema": "https://json.schemastore.org/tsconfig", | ||
| "compilerOptions": { | ||
| "composite": true, |
There was a problem hiding this comment.
You removed the composite option. If you ever plan to enable project references or incremental builds, you’ll need composite: true and the related declaration flags. Otherwise, you can drop other shared configs too:
// Optionally re-enable if you use references:
{"compilerOptions": {"composite": true, "declaration": true, /* ... */}}
|
Replaced the existing YAML-based ESLint configuration with a streamlined JavaScript flat config, upgraded various linting and TypeScript tooling versions, and updated package dependencies. Minor code cleanups include standardizing axios error handling imports, removing redundant ESLint disables, and adjusting the TS config precision. These changes modernize the linting setup and enhance developer experience without altering functional behavior. Walkthrough
Model: o4-mini | Prompt Tokens: 4721 | Completion Tokens: 1211 |
|
Updated the CI workflow to pull the Node.js version from the project's Walkthrough
Model: o4-mini | Prompt Tokens: 403 | Completion Tokens: 313 | Diff Range: abb5fde...8afabe3 |
There was a problem hiding this comment.
A supportive code review with AI helping you along. Suggestions are friendly ideas, not hard rules, so trust what works for you. You’re in charge and AI is here to help.
Model: o4-mini | Prompt Tokens: 849 | Completion Tokens: 1837 | Diff Range: abb5fde...8afabe3
… single .prettierrc.json
|
This change cleans up ESLint configuration files by removing unnecessary semicolons from import statements to match the project’s style conventions and eliminates an outdated Prettier configuration file. Walkthrough
Model: o4-mini | Prompt Tokens: 653 | Completion Tokens: 480 | Diff Range: 8afabe3...d97a38a |
There was a problem hiding this comment.
Thank you for sharing your code. This AI-assisted review offers suggestions and observations to consider. Some insights are possibilities rather than guarantees, so trust your judgment and choose what works best for your goals. You’re guiding the process—AI is here to support and empower you.
Model: o4-mini | Prompt Tokens: 1967 | Completion Tokens: 2026 | Diff Range: 8afabe3...d97a38a
There was a problem hiding this comment.
Here's a friendly code review enhanced by AI assistance. These suggestions are insights and recommendations rather than definitive requirements, so please apply what feels most valuable to your project. You remain in full control of all technical decisions—AI simply provides additional perspective to support your development process.
Model: anthropic--claude-4-sonnet | Prompt Tokens: 8347 | Completion Tokens: 1517
| let lineNo = 0 | ||
|
|
||
| const fileStatus = file.deleted ? "deleted" : file.new ? "added" : file.from !== file.to ? "renamed" : "modified" // eslint-disable-line sonarjs/no-nested-conditional | ||
| const fileStatus = file.deleted ? "deleted" : file.new ? "added" : file.from !== file.to ? "renamed" : "modified" // eslint-disable-line sonarjs/no-nested-conditional, unicorn/no-nested-ternary |
There was a problem hiding this comment.
Consider extracting the file status determination logic into a separate function for better readability and testability.
function determineFileStatus(file: ParsedDiff): string {
if (file.deleted) return "deleted"
if (file.new) return "added"
if (file.from !== file.to) return "renamed"
return "modified"
}
const fileStatus = determineFileStatus(file)| comparison.forEach(file => { | ||
| const fileName = file.from === file.to ? file.from : `${file.from} → ${file.to}` | ||
| const fileStatus = file.deleted ? "deleted" : file.new ? "added" : file.from !== file.to ? "renamed" : "modified" // eslint-disable-line sonarjs/no-nested-conditional | ||
| const fileStatus = file.deleted ? "deleted" : file.new ? "added" : file.from !== file.to ? "renamed" : "modified" // eslint-disable-line sonarjs/no-nested-conditional, unicorn/no-nested-ternary |
There was a problem hiding this comment.
Same suggestion as in pr-review: Consider extracting the file status determination logic into a separate function for better code reusability.
function determineFileStatus(file: ParsedDiff): string {
if (file.deleted) return "deleted"
if (file.new) return "added"
if (file.from !== file.to) return "renamed"
return "modified"
}
const fileStatus = determineFileStatus(file)* Upgraded eslint to version 9 * Refactor error handling to use isAxiosError from axios instead of axios.isAxiosError * Remove composite option from tsconfig.json * Remove unused ESLint disable directives * Update Node.js setup to use version file for consistency * Formatting * Remove individual Prettier configuration files and consolidate into a single .prettierrc.json
Migrate ESLint configuration file from the eslintrc format (typically configured in .eslintrc.js or .eslintrc.json files) to the new flat config format (typically configured in an eslint.config.js file).
This change modernizes the project's linting infrastructure by migrating from ESLint v8 legacy configuration to the newer flat config format with ESLint v9, while also standardizing Node.js version management across the codebase and cleaning up dependencies.
Walkthrough
.eslintrc.ymlto modern flat config format (eslint.config.js) with updated plugin imports and rule definitions.tool-versionsfile reference in GitHub Actions workflow for consistent version managementModel: anthropic--claude-4-sonnet | Prompt Tokens: 6322 | Completion Tokens: 241