fix: known types issue #4
Conversation
🦋 Changeset detectedLatest commit: 7375ad5 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 |
|
Warning Rate limit exceeded@JounQin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Signed-off-by: JounQin <admin@1stg.me>
|
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: |
There was a problem hiding this comment.
Pull Request Overview
This PR refines the legacy resolver type definitions to be more flexible and introduce specialized resolver object interfaces.
- Made
resolveandresolveImportmethods optional inLegacyResolver - Changed
optionsto a genericunknownon the base and added three specialized resolver object interfaces - Introduced a union type for
LegacyResolverObjectincluding both base and specialized variants
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to d74bf7f in 1 minute and 6 seconds. Click for details.
- Reviewed
59lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft 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. src/types.ts:76
- Draft comment:
Changed 'resolve' and 'resolveImport' to optional. Ensure that consumers check for their existence to avoid runtime errors. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. src/types.ts:87
- Draft comment:
Renamed LegacyResolverObject to LegacyResolverObjectBase. Confirm that all external references have been updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to confirm that all external references have been updated, which violates the rule against asking the author to confirm or double-check things. It does not provide a specific code suggestion or point out a specific issue.
3. src/types.ts:89
- Draft comment:
Replaced union of specific options types with 'unknown'. This reduces type safety; if flexibility is needed, consider documenting expected shapes or retaining a union type. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. src/types.ts:96
- Draft comment:
New specialized resolver object interfaces (LegacyNodeResolverObject, LegacyTsResolverObject, LegacyWebpackResolverObject) were added. Verify that the union type discriminates correctly (using the 'name' field) for proper type narrowing. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that the union type discriminates correctly, which is against the rules. It does not provide a specific suggestion or point out a specific issue with the code.
Workflow ID: wflow_ggjKIfpTAAqTOI83
You can customize 
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 53 53
Branches 16 16
=========================================
Hits 53 53 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Deploy preview for eslint-import-context ready! ✅ Preview Built with commit 7375ad5. |
📊 Package size report 2%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
size-limit report 📦
|
Important
Make
resolveandresolveImportoptional inLegacyResolverand refineLegacyResolverObjecttypes for better flexibility and clarity.resolveandresolveImportinLegacyResolverare now optional, allowing more flexible resolver configurations.LegacyResolverObjectsplit intoLegacyNodeResolverObject,LegacyTsResolverObject, andLegacyWebpackResolverObjectfor clearer type definitions.optionsinLegacyResolverObjectBasechanged tounknownto accommodate various resolver options.This description was created by
for d74bf7f. You can customize this summary. It will automatically update as commits are pushed.