-
-
Notifications
You must be signed in to change notification settings - Fork 69
fix: add more extension aliases for ts source/declaration files #442
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
Conversation
🦋 Changeset detectedLatest commit: d6facb6 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 |
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.
Pull Request Overview
This pull request adds more extension aliases for TypeScript source and declaration files, updating both implementation and documentation. Key changes include:
- Adding additional alias mappings for .ts, .tsx, .cts, and .mts in the source code.
- Updating the error log message in the resolver implementation.
- Updating unit tests and README documentation to support the new aliases.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tests/unit/unit.spec.ts | Refactored test structure; added a new test for resolving .d.ts files. |
src/index.ts | Updated the error log message for consistency. |
src/constants.ts | Extended the alias mapping for several file extensions. |
README.md | Updated extension alias documentation to align with implementation. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
size-limit report 📦
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #442 +/- ##
==========================================
+ Coverage 63.58% 64.45% +0.86%
==========================================
Files 6 6
Lines 368 377 +9
Branches 48 48
==========================================
+ Hits 234 243 +9
Misses 134 134 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Caution
Changes requested ❌
Reviewed everything up to 06c2d63 in 2 minutes and 8 seconds. Click for details.
- Reviewed
116
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
9
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. README.md:282
- Draft comment:
Please verify that the '.tsx' alias intentionally omits a '.js' entry, despite the comment suggesting it can be compiled as '.js'. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to verify their intention regarding the omission of a.js
entry for a.tsx
alias. This falls under asking the author to confirm their intention, which is not allowed according to the rules.
2. src/constants.ts:41
- Draft comment:
Check if the '.tsx' alias should include '.js' as noted in the comment for consistency. - Reason this comment was not posted:
Marked as duplicate.
3. src/index.ts:53
- Draft comment:
Renamed error log message to 'unrs-resolver error:'. Ensure that all documentation remains consistent with this updated name. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. tests/unit/unit.spec.ts:41
- Draft comment:
New test added for resolving '.d.ts' from a '.ts' extension. Verify this behavior meets the intended fix for issue Unable to resolve./foo.ts
to./foo.d.ts
since 3.9.0 #429. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. README.md:287
- Draft comment:
The '.tsx' alias mapping omits '.js' despite the comment indicating it can compile to .js. Consider adding '.js' if that behavior is desired. - Reason this comment was not posted:
Marked as duplicate.
6. src/index.ts:53
- Draft comment:
Updated log message to 'unrs-resolver error:' for clarity. This change looks fine. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. tests/unit/unit.spec.ts:41
- Draft comment:
Good test coverage for the new extension aliases. The added test for resolving '.d.ts' via a '.ts' import clearly verifies issue Unable to resolve./foo.ts
to./foo.d.ts
since 3.9.0 #429. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
8. README.md:51
- Draft comment:
Typographical error: In the sentence "After version 2.0.0,.d.ts
will take higher priority then normal.js
/.jsx
files", the word "then" should be "than". Please update this for correctness. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. src/index.ts:146
- Draft comment:
Typographical error: there's a duplicate 'the' in the log message (", trying from the the nearest one"). Consider removing one instance of 'the' to improve clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_YYkD03P1cPmgGklS
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
d6facb6
to
e371f47
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.
Important
Looks good to me! 👍
Reviewed e371f47 in 34 seconds. Click for details.
- Reviewed
126
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
13
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/late-elephants-confess.md:2
- Draft comment:
Changeset looks fine. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. README.md:285
- Draft comment:
Extension alias for '.ts' is added; ensure documentation matches implementation. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. README.md:287
- Draft comment:
Extension alias for '.tsx' now includes '.js' as a fallback; aligns with source, good. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. src/constants.ts:39
- Draft comment:
New extension alias entries for '.ts', '.tsx', '.cts', and '.mts' are added consistently. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. src/index.ts:53
- Draft comment:
Updated log message now references 'unrs-resolver'; ensure consistency with external logs. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
6. tests/unit/unit.spec.ts:41
- Draft comment:
New test added to ensure '.d.ts' correctly resolves for '.ts' extension; test covers issue 429. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. .changeset/late-elephants-confess.md:1
- Draft comment:
Changeset file is clear and correctly documents the patch update. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
8. README.md:282
- Draft comment:
Updated extensionAlias mapping now includes '.ts', '.tsx', '.cts' and '.mts'. This aligns with the reported issue fix. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
9. src/constants.ts:31
- Draft comment:
Extension aliases updated to include the new TS-related keys; mapping is consistent with README and appears correct. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
10. src/index.ts:53
- Draft comment:
Updated log message to 'unrs-resolver error:' to better reflect the resolver being used. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
11. tests/unit/unit.spec.ts:41
- Draft comment:
New test for resolving '.d.ts' from a '.ts' source file correctly validates the bug fix. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
12. README.md:51
- Draft comment:
Typographical error: In the sentence 'After version 2.0.0,.d.ts
will take higher priority then normal.js
/.jsx
files...', 'then' should be replaced with 'than'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. src/index.ts:146
- Draft comment:
Typo: The log message currently reads "trying from the the nearest one". Consider removing one occurrence of 'the' to correct it. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_CjJxXqeneFik0P7I
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
@coderabbitai review |
WalkthroughThis change updates the TypeScript import resolver to improve its handling of extension aliases, specifically for Changes
Sequence Diagram(s)sequenceDiagram
participant Importer
participant Resolver
participant FileSystem
Importer->>Resolver: import './foo.ts'
Resolver->>FileSystem: Check for './foo.ts'
alt Not found
Resolver->>FileSystem: Check for './foo.d.ts'
alt Found
Resolver-->>Importer: Resolve to './foo.d.ts'
else Not found
Resolver-->>Importer: Not found
end
else Found
Resolver-->>Importer: Resolve to './foo.ts'
end
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🧰 Additional context used🧬 Code Graph Analysis (2)src/index.ts (1)
tests/unit/unit.spec.ts (1)
🔇 Additional comments (13)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Actions performedReview triggered.
|
@coderabbitai Please you |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@JounQin I understand that you're requesting the use of Yarn v4 for installation, as the current Yarn v1.22.22 doesn't properly support the 🏁 Script executed:
Length of output: 361 @JounQin Thanks for your suggestion! I checked our environment and found that we're actually already using Yarn v4.9.1 with Node.js v22.9.0, and corepack 0.29.3 is available:
Despite having Yarn v4, it seems there might still be an issue with how the workspace protocol is being used in your project. The warning about "Resolution field 'workspace:*' has an invalid version entry" suggests there could be a configuration issue in your package.json. Let me know if you'd like me to examine your package.json files to better understand the workspace configuration and provide more specific guidance. |
| datasource | package | from | to | | ---------- | --------------------------------- | ----- | ----- | | npm | eslint-import-resolver-typescript | 4.3.1 | 4.3.4 | ## [v4.3.4](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#434) ##### Patch Changes - [#442](import-js/eslint-import-resolver-typescript#442) [`57611d9`](import-js/eslint-import-resolver-typescript@57611d9) Thanks [@JounQin](https://github.com/JounQin)! - fix: add more extension aliases for ts source/declaration files - [#444](import-js/eslint-import-resolver-typescript#444) [`bd45fcd`](import-js/eslint-import-resolver-typescript@bd45fcd) Thanks [@JounQin](https://github.com/JounQin)! - fix(deps): bump `unrs-resolver` which resolves [#406](import-js/eslint-import-resolver-typescript#406), [#409](import-js/eslint-import-resolver-typescript#409), [#437](import-js/eslint-import-resolver-typescript#437) ## [v4.3.3](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#433) ##### Patch Changes - [#433](import-js/eslint-import-resolver-typescript#433) [`834b11e`](import-js/eslint-import-resolver-typescript@834b11e) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `unrs-resolver` to v1.6.0 ## [v4.3.2](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#432) ##### Patch Changes - [#427](import-js/eslint-import-resolver-typescript#427) [`dabba8e`](import-js/eslint-import-resolver-typescript@dabba8e) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `unrs-resolver` to v1.4.1
close #429
cc @silverwind
Important
Add more extension aliases for TypeScript files and update tests and logging accordingly.
.ts
,.tsx
,.cts
, and.mts
insrc/constants.ts
andREADME.md
.unit.spec.ts
to verify.d.ts
resolution with.ts
extension.unrsResolve()
insrc/index.ts
to correct resolver name.This description was created by
for e371f47. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Bug Fixes
.ts
,.tsx
,.cts
, and.mts
files and their declaration or compiled variants.Documentation
Tests
.ts
imports to.d.ts
files and improved test structure for maintainability.