fix(parser): error when both projectService and project are set
#11333
Conversation
|
Thanks for the PR, @MariaSolOs! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit a8c4dc1.
☁️ Nx Cloud last updated this comment at |
MariaSolOs
force-pushed
the
parser-config
branch
from
cfe374a to
d4979a5
Compare
MariaSolOs
force-pushed
the
parser-config
branch
from
d4979a5 to
a8c4dc1
Compare
There was a problem hiding this comment.
These are just notes from quick preliminary review. I think we have a good path forward but should definitely let other team folks weigh in in #11319.
For now, I'll switch this PR to draft so it doesn't show up on the queue. But we can definitely un-draft it once the issue's discussion is done.
| @@ -145,6 +145,12 @@ export function createParseSettings( | |||
| tsconfigRootDir, | |||
| }; | |||
|
|
|||
| if (parseSettings.projectService && tsestreeOptions.project) { | |||
| throw new Error( | |||
| 'Cannot use both projectService and project parser options.', | |||
There was a problem hiding this comment.
[Docs] I think users who are enabling both are probably not super familiar with them. We'll need to guide them a bit more. Maybe something like...
| 'Cannot use both projectService and project parser options.', | |
| 'Enabling 'project' does nothing when 'projectService' is already enabled. You can remove 'project'.', |
Note that we can't assume project: true rather than string / string[]. Example from the wild, https://sourcegraph.com/github.com/software-mansion/smelter/-/blob/ts/eslint.config.js?L28-35
There was a problem hiding this comment.
[Testing] Looks like our own test infrastructure is getting in the way. Both are set because that was easier to do in CI:
typescript-eslint/.github/workflows/ci.yml
Lines 256 to 260 in 702cea8
Maybe we should temporarily give users a way to disable the new error? TYPESCRIPT_ESLINT_IGNORE_PROJECT_AND_PROJECT_SERVICE_ERROR or some similarly unwieldy name? I'll post in #11319.
|
@JoshuaKGoldberg Thanks for the review! I'll wait for further thoughts from other maintainers before I continue working on this :) |
PR Checklist
projectServiceandprojectare set #11319Overview
As mentioned in the issue, the docs aren't clear of what the parser will use when both
projectServiceandprojectare set. When such case is detected, I think it's better to be explicit about the misconfiguration and error as early as possible.