Skip to content

feat: Add bulk suppressions feature for Node.js API #133

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sushichan044
Copy link

Summary

This RFC proposes extending the bulk suppressions feature, introduced in ESLint 9.24.0 for CLI usage, to be available through the Node.js API via the ESLint and LegacyESLint classes.

Related Issues

Copy link

linux-foundation-easycla bot commented May 5, 2025

CLA Signed


The committers listed above are authorized under a signed CLA.

@sushichan044
Copy link
Author

Specific code examples for LegacyESLint are WIP, but will be added soon.

@mdjermanovic mdjermanovic added the Initial Commenting This RFC is in the initial feedback stage label May 6, 2025
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the RFC! I left some initial questions about the design.

used. Be sure to define any new terms in this section.
-->

This proposal integrates the existing bulk suppression functionality into the `ESLint` and `LegacyESLint` Node.js API classes by leveraging the internal `SuppressionsService`. A new `suppressionsLocation` option is introduced in the constructors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ESLint CLI uses ESLint API, and part of the bulk suppressions functionality will now be implemented directly in the API, will there be any changes to the ESLint CLI code (https://github.com/eslint/eslint/blob/main/lib/cli.js)? For example, perhaps removing some code that is now implemented in the API.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the CLI uses the API, so on the CLI side need to remove the calls of SuppressionsService.
I forgot to write it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed: c5b4134

Comment on lines +19 to +22
Currently, the bulk suppression feature introduced in ESLint 9.24.0 is only available via the CLI.
This leads to inconsistencies when ESLint is used programmatically via its Node.js API, such as in IDE integrations.

This leads to inconsistencies when ESLint is used programmatically. Violations suppressed using `eslint-suppressions.json` (especially when using a custom location via the CLI) might not be recognized when using the Node.js API, leading to incorrect error reporting in environments like IDEs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppose someone has a task to fix previously suppressed violations. How will they do this, since the violations no longer appear in their IDE?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation for using bulk suppression may be “I want to ignore existing errors and give priority to activating new errors”.
With that assumption, existing errors that should be ignored should not be shown in the IDE either.

To work on reducing the number of ignored errors, we can start by check the suppressed errors in eslint-suppressions.json.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdjermanovic I think this is the same use case as when running on the CLI. Because the CLI automatically picks up the eslint-suppressions.json file, it's necessary to edit, move, or delete the file if your task is to clean up suppressed violations.

@nzakas
Copy link
Member

nzakas commented May 7, 2025

@sushichan044 please take a moment to sign the CLA (see first comment).

nzakas
nzakas previously approved these changes May 7, 2025
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks like a good plan to me. Nice work.

Comment on lines +19 to +22
Currently, the bulk suppression feature introduced in ESLint 9.24.0 is only available via the CLI.
This leads to inconsistencies when ESLint is used programmatically via its Node.js API, such as in IDE integrations.

This leads to inconsistencies when ESLint is used programmatically. Violations suppressed using `eslint-suppressions.json` (especially when using a custom location via the CLI) might not be recognized when using the Node.js API, leading to incorrect error reporting in environments like IDEs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdjermanovic I think this is the same use case as when running on the CLI. Because the CLI automatically picks up the eslint-suppressions.json file, it's necessary to edit, move, or delete the file if your task is to clean up suppressed violations.

-->

- Specific implementation examples for the `LegacyESLint` class.
- Should functionality equivalent to CLI flags like `--suppress-all` or `--suppress-rule` be supported via separate API methods in the future?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. I think the main use case is people will create and manage a suppressions file using the CLI. The IDE use case is more an extension of that rather than a replacement.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nzakas
I was relieved because I had the same opinion.
It was an open question to confirm if our understanding was aligned.

Given this direction, I believe it would be better to slightly modify the RFC's title and content to focus on the scope of considering bulk suppressions when running Lint via the Node.js API.
The reason is that with the current content, it might take time for future discussions referencing this RFC to understand that handling the suppression content itself is out of scope.

How do you think?

Co-authored-by: Nicholas C. Zakas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Initial Commenting This RFC is in the initial feedback stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants