Convert to eslint 9 #4056

robertbrignull · 2025-06-18T16:24:13Z

Upgrades eslint to version 9, as well as upgrading a few plugins.

I've tried to convert the old config as closely as I could, but I've removed a few plugins that weren't working well because they haven't received updates in a couple of years. It's also very possible I've misunderstood bits of the config!

I can now run npm run lint locally and hopefully it'll pass in CI too. I have fixed a few rules where it was simple or obvious to do, but for most rules I have disabled them as there were thousands of errors. After this is merged we'll want a secondary effort to try to re-enable rules as much as feasible.

I've tried to keep the list of commits as clear and understandable as possible, so hopefully you can review this commit-by-commit.

Copilot

Pull Request Overview

This PR upgrades ESLint from version 8 to version 9, along with updating several related plugins. The upgrade involves transitioning from the legacy .eslintrc.js configuration format to the new flat config format in eslint.config.mjs. Many lint rules have been temporarily disabled to address thousands of errors, with the plan to re-enable them incrementally in future work.

Replaces legacy ESLint configuration with flat config format
Updates ESLint plugins and removes outdated ones
Removes unnecessary non-null assertions and type casts throughout the codebase
Cleans up disabled ESLint comments that are no longer needed

Reviewed Changes

Copilot reviewed 39 out of 41 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`package.json`	Updates ESLint and plugin versions, removes outdated plugins
`eslint.config.mjs`	New flat config replacing old `.eslintrc.js` with updated rule configurations
`.eslintrc.js`	Removed legacy ESLint configuration file
`.eslintignore`	Removed in favor of global ignores in flat config
Multiple source files	Removes unnecessary non-null assertions and type casts flagged by new rules

Copilot · 2025-08-04T16:13:56Z

extensions/ql-vscode/src/common/mock-gh-api/request-handlers.ts

      request.response.body = await readFile(
        join(scenarioDirPath, request.response.body.substring(5)),
+        "utf8",
      );


The readFile function is being called with an explicit encoding parameter "utf8", but the function signature and context suggest this might be reading binary data. Consider verifying that UTF-8 encoding is appropriate for all file types being read here, or handle encoding based on file type.

Suggested change

);

const filePathToRead = join(scenarioDirPath, request.response.body.substring(5));

const fileBuffer = await readFile(filePathToRead);

// Convert to string only for text files

if (filePathToRead.endsWith(".txt") || filePathToRead.endsWith(".json")) {

request.response.body = fileBuffer.toString("utf8");

} else {

request.response.body = fileBuffer;

}

Copilot · 2025-08-04T16:13:56Z

extensions/ql-vscode/src/local-queries/query-contents.ts

  if (!language) {
-    const dbschemeBase = basename(dbscheme) as keyof typeof dbSchemeToLanguage;
+    const dbschemeBase = basename(dbscheme);
    language = dbSchemeToLanguage[dbschemeBase];


Removing the type assertion as keyof typeof dbSchemeToLanguage could cause a runtime error if dbschemeBase is not a valid key in the dbSchemeToLanguage object. The indexing operation may return undefined, which could lead to unexpected behavior.

Suggested change

language = dbSchemeToLanguage[dbschemeBase];

if (dbschemeBase in dbSchemeToLanguage) {

language = dbSchemeToLanguage[dbschemeBase];

}

could cause a runtime error

I think the code being removed is entirely compile-time, so I'm going to ignore this copilot suggestion about runtime errors.

Copilot · 2025-08-04T16:13:57Z

extensions/ql-vscode/test/mock-memento.ts

  public get<T>(key: string, defaultValue: T): T;
-  public get(key: any, defaultValue?: any): T | T | undefined {
+  public get(key: any, defaultValue?: any): T | undefined {
    return this.map.get(key) || defaultValue;


The return type should be T | undefined when no default value is provided, or T when a default value is provided. The current signature removes the overload distinction that was present in the original interface, potentially breaking type safety.

Suggested change

return this.map.get(key) || defaultValue;

public get<T>(key: string, defaultValue?: T): T | undefined {

if (this.map.has(key)) {

return this.map.get(key) as T;

}

if (defaultValue !== undefined) {

return defaultValue;

}

return undefined;

koesie10

It makes sense to me to fix the other ESLint violations in follow-up PRs. These changes look good to me, my main concern is regarding the request handler where I don't think we should be reading the file as UTF-8.

extensions/ql-vscode/src/common/mock-gh-api/request-handlers.ts

extensions/ql-vscode/src/view/common/SuggestBox/SuggestBox.tsx

koesie10 · 2025-08-06T11:56:31Z

extensions/ql-vscode/test/vscode-tests/test-dispose-handler.ts

+    (disposable as any).onDidExpandElement &&
+    (disposable as any).onDidCollapseElement &&
+    (disposable as any).reveal
  ) {


I think this suggestion from Copilot makes sense, assuming the in checks are also true for an actual TreeViewer.

Co-authored-by: Koen Vlaswinkel <[email protected]>

koesie10

LGTM, thanks!

robertbrignull requested review from a team and Copilot June 18, 2025 16:24

robertbrignull requested a review from a team as a code owner June 18, 2025 16:24

This comment was marked as outdated.

Sign in to view

This was referenced Jul 22, 2025

Remove duplicate enum #4085

Merged

Avoid specifying error type since the default is unknown #4086

Merged

robertbrignull force-pushed the robertbrignull/eslint-9 branch from 76dfe00 to d77537a Compare August 4, 2025 15:46

Copilot AI review requested due to automatic review settings August 4, 2025 15:59

This comment was marked as outdated.

Sign in to view

robertbrignull added 13 commits August 4, 2025 16:12

Convert to eslint 9

f8266b9

Include all storybook files

20dc937

Ignore unlintable files

bc5411a

Disable rules that can't be fixed right now

1d44f6c

Remove unnecessary eslint-disable-next-line directives

010d351

Fix import from gulp-replace

4bd054d

Fix cases of @typescript-eslint/no-for-in-array

b638e41

Fix @typescript-eslint/no-unsafe-enum-comparison

2796dda

Run eslint --fix

eb4a10b

Fix @typescript-eslint/prefer-promise-reject-errors by using asError

a190b45

Fix @typescript-eslint/no-redundant-type-constituents

25007a9

Decode file contents as utf8

7b1ac4d

Parse MAX_QUERIES as number

195c7e0

Copilot AI review requested due to automatic review settings August 4, 2025 16:13

robertbrignull force-pushed the robertbrignull/eslint-9 branch from f568822 to 195c7e0 Compare August 4, 2025 16:13

Copilot AI reviewed Aug 4, 2025

View reviewed changes

robertbrignull added 3 commits August 4, 2025 16:17

Fix type in MockMemento

b40a99b

Extract buffer from NonSharedBuffer

ef98797

Fix importing css styles

320df4d

robertbrignull force-pushed the robertbrignull/eslint-9 branch from 6089c83 to 320df4d Compare August 4, 2025 16:40

koesie10 reviewed Aug 6, 2025

View reviewed changes

robertbrignull and others added 4 commits August 6, 2025 16:35

Apply suggestion from @koesie10

87b8413

Co-authored-by: Koen Vlaswinkel <[email protected]>

Don't decode mock request body as utf8

f58d76b

Add isTreeView type checker function

d20f8a6

Fix linting by removing newline

f656545

koesie10 approved these changes Aug 11, 2025

View reviewed changes

robertbrignull merged commit 75cf570 into main Aug 11, 2025
33 of 34 checks passed

robertbrignull deleted the robertbrignull/eslint-9 branch August 11, 2025 15:39

robertbrignull mentioned this pull request Aug 11, 2025

Bump eslint from 8.56.0 to 9.17.0 in /extensions/ql-vscode #3875

Closed

-      );
+      const filePathToRead = join(scenarioDirPath, request.response.body.substring(5));
+      const fileBuffer = await readFile(filePathToRead);
+      // Convert to string only for text files
+      if (filePathToRead.endsWith(".txt") || filePathToRead.endsWith(".json")) {
+        request.response.body = fileBuffer.toString("utf8");
+      } else {
+        request.response.body = fileBuffer;
+      }

-    return this.map.get(key) || defaultValue;
+  public get<T>(key: string, defaultValue?: T): T | undefined {
+    if (this.map.has(key)) {
+      return this.map.get(key) as T;
+    }
+    if (defaultValue !== undefined) {
+      return defaultValue;
+    }
+    return undefined;

Uh oh!

Convert to eslint 9 #4056

Convert to eslint 9 #4056

Uh oh!

Conversation

robertbrignull commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

robertbrignull Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

koesie10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

koesie10 Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

koesie10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robertbrignull commented Jun 18, 2025 •

edited

Loading