-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
fix(typescript-estree): pass extraFileExtensions to projectService #9051
fix(typescript-estree): pass extraFileExtensions to projectService #9051
Conversation
Thanks for the PR, @alfredringstad! 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 failed.
|
5e3d1c0
to
88c1ab6
Compare
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 2a460d5. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 12 targets
Sent with 💌 from NxCloud. |
88c1ab6
to
c21ca49
Compare
c21ca49
to
a19ab95
Compare
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.
Source code changes look great to me! And checking that the config file path is found as a form of testing is very snazzy. Nicely done. 😄
Just requesting changes on docs and a few testing things that I'm open to being pushed back on - what do you think?
packages/typescript-estree/tests/lib/useProgramFromProjectService.test.ts
Outdated
Show resolved
Hide resolved
62b24c6
to
368d020
Compare
368d020
to
18beb06
Compare
Thanks for the feedback @JoshuaKGoldberg! I've implemented your suggestions. Seems like CI failure is due to issues in the main v8 branch, the typescript-estree tests seem to work fine. |
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.
[Praise] 💯
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.
This is great, thanks!
Normally I'd wait for a bit to let someone else review but I would like to unblock folks who need the extra file extensions from using the project.
PR Checklist
Overview
Uses
setHostConfiguration
to passextraFileExtensions
to project service, when usingEXPERIMENTAL_useProjectService
. This fixes an issue where project service would not detect the appropriate tsconfig file when using non-default file extensions.The tests are checking that the configFilePath is set in the compiler options. If it's not, it's an indication that the default inferred project is used. I duplicated the test cases used for the non-experimental projects option.