-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Update eslint from 8 to 9. #14157
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
base: main
Are you sure you want to change the base?
Update eslint from 8 to 9. #14157
Conversation
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.
Pull request overview
This PR updates ESLint from version 8 to version 9, transitioning to the new flat configuration format. The update involves migrating from .eslintrc.js to eslint.config.js and removing numerous inline ESLint disable directives that are no longer needed with the updated configuration.
Changes:
- Migrated ESLint configuration from legacy
.eslintrc.jsformat to new flat configeslint.config.js - Updated ESLint and TypeScript ESLint dependencies to latest versions
- Removed obsolete inline ESLint disable comments throughout the codebase
- Refactored several async functions to synchronous wrappers with void-ed async IIFEs to comply with new no-floating-promises rules
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Extension/package.json | Updated ESLint dependencies to v9 and TypeScript ESLint to v8, added commonjs module type, removed -c flag from lint script |
| Extension/eslint.config.js | Created new flat config file with TypeScript, JSDoc, and import plugin rules |
| Extension/.eslintrc.js | Removed legacy ESLint configuration file |
| Extension/.eslintignore | Removed legacy ignore file (now handled in flat config) |
| Extension/src/LanguageServer/client.ts | Converted async functions to sync wrappers with void-ed async IIFEs, removed eslint-disable comments |
| Extension/src/LanguageServer/cppBuildTaskProvider.ts | Converted async open method to sync with void-ed async IIFE |
| Extension/src/Debugger/configurationProvider.ts | Removed no-prototype-builtins and no-dynamic-delete disable comments |
| Extension/src/common.ts | Removed no-control-regex disable comments |
| Extension/src/Utility/* | Removed various eslint-disable comments for unused-vars, ban-types, no-constant-condition |
| Extension/src/SSH/sshCommandToConfig.ts | Removed no-constant-condition disable comment |
| Extension/test/* | Removed eslint-disable comments for unused-vars and no-dynamic-delete |
| Extension/ui/settings.ts | Removed no-unused-vars disable comment |
| Extension/.scripts/* | Removed no-unused-vars and no-prototype-builtins disable comments, added one new disable for no-misused-promises |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment was marked as resolved.
This comment was marked as resolved.
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.
Pull request overview
Copilot reviewed 30 out of 33 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 30 out of 33 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| export async function all() { | ||
| await rimraf(...(await getModifiedIgnoredFiles()).filter(each => !each.includes('node_modules'))); | ||
| await rimraf(...(await getModifiedIgnoredFiles()).filter((each): each is string => each !== undefined && !each.includes('node_modules'))); |
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.
Why does it want each is string annotations on the functions? getModifiedIgnoredFiles returns string[] so there's no doubt concerning its type.
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.
Hmm...but it gives a linter error if I remove it.
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.
Copilot thinks the code is good. And other fixes it says are not as good.
This will create a TypeScript error because the filter won't narrow the type from [(string | undefined)[]
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.
Also, it's getModifiedIgnoredFiles(): Promise<(string | undefined)[]>
This was mostly done by Copilot Agent mode.