Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix(type-utils): treat custom type roots as external#6870
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix(type-utils): treat custom type roots as external#6870
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for the PR,@RebeccaStevens! 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 onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedApr 8, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
RebeccaStevens commentedApr 8, 2023
nx-cloudbot commentedApr 8, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
marekdedic commentedApr 8, 2023
marekdedic commentedApr 8, 2023
Adding a test for this would probably be quite convoluted, right? |
JoshuaKGoldberg 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.
Yip, similar to the other one, seems reasonable but let's test 👍
marekdedic left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
| */ | ||
| intrinsicName?:string; | ||
| } | ||
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.
Adding our own type declarations for the internalsourceFileToPackageName seems fishy to me, but that's a decision the maintainers need to do.
I presume you are aware of#6861 - however, there has been no response in the corresponding TS issue....
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.
Yeah for now I think it's fine. If they change that API we should see breakages in the TS version upgrade PR.
Uh oh!
There was an error while loading.Please reload this page.
JoshuaKGoldberg commentedJul 7, 2023
@RebeccaStevens I'm hoping to get through the last bits of v6 work today since we're set to launch on Monday. I think that'll just entail merging |
JoshuaKGoldberg commentedJul 8, 2023
Hmm, yeah, these unit tests are tough. I ran out of time tonight. Will try again tomorrow. |
JoshuaKGoldberg commentedJul 8, 2023
😕 I can't figure this out.9b49480 shows a unit test that I would think should populate To repro:
|
RebeccaStevens commentedJul 9, 2023
Yeah, I gave up on the test too. I came to the conclusion that internal test API thingy isn't setup to do these sort of tests. I didn't want to open that can of worms of editing that. If I remember right, local tests I tried using a separate repo passed. I can't really turn those into unit test though. |
JoshuaKGoldberg commentedJul 10, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This was unintentionally auto-closed when we merged the |
JoshuaKGoldberg 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.


Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
typeMatchesSpecifierPackageSpecifiershould match custom type roots #6868Overview