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

feat(linter): Support --print-config feature of ESLint #18260

Merged

Conversation

tonyganchev
Copy link
Contributor

The change introduces a new option to the linter plugin schema: printConfig, accepting a file name for which the plugin to output its applicable ESLint configuration.

Current Behavior

There is no support for --print-config.

Expected Behavior

When --printConfig is passed on the command line as a parameter to nx lint ... or printConfig is specified in the project.json the linter will show the effective ESLint configuration applicable to the specified file and not proceed with linting the project. Operation would complete successfully.

Related Issue(s)

none

Fixes #

@vercel
Copy link

vercel bot commented Jul 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 29, 2023 5:17am

@meeroslav
Copy link
Contributor

Hi @tonyganchev,

Thank you for creating this PR.

The purpose of eslint executor is to enable us to control the linting and achieve nx features such as orchestration, task running, caching, affected graph, etc. We call eslint CLI directly when invoking the linting.

Printing out configuration doesn't need any of that and you can easily run it as npx eslint --print-config name-of-the-file.

Why would you need this as a parameter in the executor?

@meeroslav meeroslav self-assigned this Jul 25, 2023
@tonyganchev
Copy link
Contributor Author

Hi @tonyganchev,

Thank you for creating this PR.

The purpose of eslint executor is to enable us to control the linting and achieve nx features such as orchestration, task running, caching, affected graph, etc. We call eslint CLI directly when invoking the linting.

Printing out configuration doesn't need any of that and you can easily run it as npx eslint --print-config name-of-the-file.

Why would you need this as a parameter in the executor?

@meeroslav looking at the code I was under the impression that the nx plugin is invoking the ESLint API directly in resolveAndInstantiateESLint() of eslint-utils.ts.

In any case I have been hitting issues where running eslint on the command line yields different results from running nx lint e.g. some files don't seem to be selected in nx-orchestrated lint. I needed a way to determine if nx was modifying the ESLint configuration / ruleset or there was a different issue. Turns out the issue was incorrect file selection in the lint target definition in the project.json which I could only spot through manual inspection i.e. looked at the config and miraculously I spotted it :)

Providing access to all capabilities of the orchestrated tools should be critical in helping developers troubleshoot integration issues, learn the nx tooling and therefore trust it.

Best
Tony

@meeroslav
Copy link
Contributor

Thank you, @tonyganchev, for clarifying that. It makes more sense now, why one would need this flag supported in our executor.

And yes, you are right; we use API, not CLI (that was a typo on my side, sorry for the confusion).

@meeroslav
Copy link
Contributor

Can you also add an E2E test, please, to https://github.com/tonyganchev/nx/blob/topic/tonyganchev/linter-print-config/e2e/linter/src/linter.test.ts that would test this flag?

@tonyganchev
Copy link
Contributor Author

Can you also add an E2E test, please, to https://github.com/tonyganchev/nx/blob/topic/tonyganchev/linter-print-config/e2e/linter/src/linter.test.ts that would test this flag?

Certainly. The latest version now includes a test for this.

The change introduces a new option to the linter plugin schema:
`printConfig`, accepting a file name for which the plugin to output its
applicable ESLint configuration.

!fixup feat(linter): re-submitting documentation
Copy link
Contributor

@meeroslav meeroslav left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@meeroslav meeroslav changed the title feat(linter): Support for --print-config feature of ESLint feat(linter): Support --print-config feature of ESLint Jul 31, 2023
@meeroslav meeroslav merged commit 30c3e99 into nrwl:master Jul 31, 2023
15 checks passed
@github-actions
Copy link

github-actions bot commented Aug 6, 2023

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
scope: linter Issues related to Eslint support in Nx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants