-
-
Notifications
You must be signed in to change notification settings - Fork 686
Initial Release of ES Lint #22663
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
base: main
Are you sure you want to change the base?
Initial Release of ES Lint #22663
Conversation
|
Thanks for the contribution! Could you flesh out the "why" and "what" of the PR description? |
riisi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timsmallwood Thanks for submitting this - it would definitely be good to have support for eslint.
I've reviewed it briefly - looking at the implementation mostly and not the tests at all yet. Not a maintainer, but I think I have an idea of what would be expected.
I have further questions about the architecture which I've not had time to look into.
- Caching: It looks like this is not implemented. Is it absolutely needed or can it be dropped for now to reduce scope?
- Multi-project/resolve handling: How would it work across independent projects with different eslint versions? Does eslint need to operate in a package/project context, or does it just run per-file? In terms of existing goals - is it more like Prettier, or more like Typescript?
| ) | ||
| ``` | ||
|
|
||
| ## Configuration Files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better here to link to the eslint docs on config file formats and just state that Pants supports them (to avoid re-documenting here, plus this doc is already quite long).
|
|
||
| ## Supported File Types | ||
|
|
||
| ESLint in Pants supports: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other types that are currently unsupported? Or is this a generic statement implying that Pants supports all the types one would want to use?
Are .jsx and .tsx files supported?
|
|
||
| ### Prettier Integration | ||
|
|
||
| ESLint and Prettier can work together. If you're using both tools, consider: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The numbering implies to me that all of these things should be done in order - I'm not sure that's the case?
Does Pants support running prettier first, then eslint by default, or we need to consider the order of pants fmt lint ... ?
I do think covering this usage is important FWIW - it's been a bit of a minefield in personal past experience to get these working together. I think if we can offer some opinionated guidance, that would be helpful.
| }; | ||
| ``` | ||
|
|
||
| **Note**: ESLint can lint TypeScript but has limited formatting capabilities for TypeScript-specific syntax. For comprehensive TypeScript formatting, consider using Prettier alongside ESLint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it might make sense to have one docs section on linting and formatting, like with Python - https://www.pantsbuild.org/2.27/docs/python/overview/linters-and-formatters - but we can leave it for now.
| ``` | ||
|
|
||
| :::tip Cache Location | ||
| ESLint will create a `.eslintcache` file to store cache data. Consider adding this to your `.gitignore`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering how this works - wouldn't this be in sandbox and not need to be ignored?
|
|
||
| wants_json = False | ||
| if getattr(eslint, "json_output", False): | ||
| # Only add json format if user did not already specify a formatter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can json just be a default option instead?
| if wants_json: | ||
| try: | ||
| parsed = json.loads(stdout or "[]") | ||
| if isinstance(parsed, list): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No else?
| summary = f"ESLint summary: {errors} errors, {warnings} warnings\n\n" | ||
| stdout = summary + stdout | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to swallow the exception?
| @@ -0,0 +1,25 @@ | |||
| # Copyright 2025 Pants project contributors (see CONTRIBUTORS.md). | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, following the pattern from the other javascript/typescript plugins, that the most of the eslint code should not be located under backend/experimental - only register.py -- see e.g. prettier.
| ), | ||
| ) | ||
|
|
||
| extra_dev_dependencies = StrListOption( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see, this isn't used in the current implementation?
Add comprehensive ESLint integration for JavaScript/TypeScript linting and formatting
Why
ESLint is the de facto standard linting tool for JavaScript and TypeScript projects. This PR adds complete ESLint
integration to Pants, filling a significant gap in the JavaScript/TypeScript toolchain support and enabling
consistent code quality enforcement across monorepo projects.
What
pants.backend.experimental.javascript.lint.eslintpants lintandpants fmtgoalspackage.json)
skip_eslint=Trueon individual targetsUsage