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: Implemented VSCode lint problem matching #2277

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

GrumpyMeow
Copy link

@GrumpyMeow GrumpyMeow commented Oct 7, 2024

With this PR i propose to have VSCode automatically take the output of NX-linting tasks and present the issues as VSCode problems.
This PR implements the matching at:

  • Running "affected" tasks
  • Running "many" tasks
  • Running an individual task
    When running affected/many the output-style is automatically changed to display the found issues and allow for matching.

A known issue is that the default ESLint-formatter which is used by NX will report with absolute filepaths. The absolute filepaths might not align with the current environment as NX-caching does not take the root-folder in account.

In the future this feature might be extended to also work for Stylelint and others.

@MaxKless Would you like to check this PR?

@GrumpyMeow GrumpyMeow changed the title Implemented VSCode lint problem matching feat: Implemented VSCode lint problem matching Oct 7, 2024
@MaxKless MaxKless self-requested a review October 14, 2024 03:27
@MaxKless
Copy link
Collaborator

Can you elaborate on a few things?

  • Why did you set --output-style=stream in a few cases?
  • Why if the lint targets are called something other than lint?
  • What if the lint targets use a technology other than eslint?
  • What happens if the eslint-stylish problem matcher isn't installed? Do we run into errors?

We should probably have another solution that checks the executor or makes sure that the targets where created by the @nx/eslint plugin to make sure we don't cause any issues.

Copy link
Collaborator

@MaxKless MaxKless left a comment

Choose a reason for hiding this comment

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

I think this still needs work. See comment above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants