Skip to content
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

Add support for overrideConfig in Programmable API #231

Merged
merged 4 commits into from Sep 2, 2022
Merged

Add support for overrideConfig in Programmable API #231

merged 4 commits into from Sep 2, 2022

Conversation

adidahiya
Copy link
Contributor

I used eslint-interactive recently to programmatically suppress lint rules across a large codebase. It worked well, but I found it to be a bit slow running all the enabled rules. Sometimes I want to use eslint-interactive with just a few rules enabled, and I found no way to do this with the programmatic API.

This PR adds two new properties accepted in the options object for the Core() constructor, overrideConfig and useEslintrc. These forward directly to the ESLint() constructor.

I've also adjusted the code and types a bit in core.ts to make it clear which parts of the API are forwarding to ESLint directly. Please let me know what you think, thanks.

extensions: this.config.extensions,
cwd: this.config.cwd,
this.config = {
...DEFAULT_BASE_CONFIG,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a bug where DEFAULT_BASE_CONFIG.quiet and DEFAULT_BASE_CONFIG.formatterName were only being used in the CLI, and not the programmatic API. I think my new code implements the original intention of DEFAULT_BASE_CONFIG.

Copy link
Owner

Choose a reason for hiding this comment

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

As you say, this is a bug. Thanks for fixing it.

@adidahiya
Copy link
Contributor Author

I enabled Github Actions, you can check out the builds on my branch here: https://github.com/adidahiya/eslint-interactive/actions?query=branch%3Aad%2FoverrideConfig

@mizdra
Copy link
Owner

mizdra commented Sep 1, 2022

Thanks for contributing! I'll review this pull request later!

@mizdra mizdra self-requested a review September 1, 2022 16:24
@adidahiya
Copy link
Contributor Author

There's a performance regression but I'm not sure what it's caused by: https://github.com/adidahiya/eslint-interactive/commit/1db1073b39d81b5dced85e2bdcc7b312454b2eda#commitcomment-82817209

Comment on lines +53 to +56
export type Config = Pick<
ESLint.Options,
'extensions' | 'rulePaths' | 'cache' | 'cacheLocation' | 'useEslintrc' | 'overrideConfig' | 'cwd'
> & {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice refactoring 👍

@mizdra
Copy link
Owner

mizdra commented Sep 2, 2022

The continuous benchmark is not stable and sometimes failed (ref: #202). You can safely ignore them this time! Sorry for worrying you.

I will raise the upper limit of the benchmark threshold at a later date.

@mizdra mizdra added the Type: Feature New Feature label Sep 2, 2022
Copy link
Owner

@mizdra mizdra left a comment

Choose a reason for hiding this comment

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

LGTM!

The programmable API was created just for fun, so I am glad to know that there are people using it.

@mizdra mizdra merged commit f4b03fe into mizdra:main Sep 2, 2022
@mizdra mizdra changed the title feat: add support for overrideConfig in programmatic API Add support for overrideConfig in Programmable API Sep 2, 2022
@mizdra mizdra added the Type: Bug Bug or Bug fixes label Sep 2, 2022
@mizdra
Copy link
Owner

mizdra commented Sep 2, 2022

This PR was released by v10.1.0.

@adidahiya adidahiya deleted the ad/overrideConfig branch September 6, 2022 15:06
@adidahiya
Copy link
Contributor Author

Thanks for the quick review and release @mizdra! I'll try out the release and follow up if I need more adjustments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug or Bug fixes Type: Feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants