-
Notifications
You must be signed in to change notification settings - Fork 61
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
Unify how module specifiers are visited by different declaration transformers #554
Conversation
🦋 Changeset detectedLatest commit: f00319c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
node: TNode | ||
): TNode => { | ||
const literal = getModuleSpecifier(node, typescript); | ||
if (literal) return handleImport(node, literal, context); |
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.
TODO: recheck this, I have a feeling that this had a bug (that is preserved by this PR) that skips over nested imports in nodes like this:
type Test = import('#foo').Foo<import('#bar').Bar>
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.
Hmmm, yeah, makes sense, I didn't realise that the qualifier and type arguments are children of the import type node. I expected it to be like how most ASTs represent property access, call expressions, etc.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #554 +/- ##
==========================================
+ Coverage 91.89% 92.16% +0.27%
==========================================
Files 41 41
Lines 1998 1979 -19
Branches 586 584 -2
==========================================
- Hits 1836 1824 -12
+ Misses 152 145 -7
Partials 10 10
☔ View full report in Codecov by Sentry. |
917693e
to
f73ea9e
Compare
Just a refactor PR, in preparation for the logic that will fix the issue with explicit
.ts
extensions (the one that we've discussed)