-
-
Notifications
You must be signed in to change notification settings - Fork 41
docs: add jsdoc for import-type util #307
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
docs: add jsdoc for import-type util #307
Conversation
|
WalkthroughThis update enhances the Changes
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
src/utils/import-type.tsOops! Something went wrong! :( ESLint: 9.24.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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 (
|
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: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #307 +/- ##
=======================================
Coverage 95.99% 95.99%
=======================================
Files 91 91
Lines 4744 4744
Branches 1763 1785 +22
=======================================
Hits 4554 4554
Misses 189 189
Partials 1 1 ☔ 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/utils/import-type.ts (2)
290-297
: Good basic documentation for typeTestThe JSDoc adequately describes the core function's purpose, parameters, and return value. Consider enhancing it with examples of different module types that could be returned.
/** * Returns the type of the module. * + * Possible return types: 'internal', 'absolute', 'builtin', 'parent', 'index', + * 'sibling', 'external', or 'unknown'. * * @param name The name of the module to check * @param context The context of the rule * @param path The path of the module to check * @returns The type of the module */
336-342
: Clear importType documentationThe JSDoc effectively documents this exported wrapper function. For completeness, you might consider mentioning that it resolves the module path before passing it to typeTest.
/** * Returns the type of the module. + * + * This function is a wrapper around typeTest that first resolves + * the module path using the resolve utility. * * @param name The name of the module to check * @param context The context of the rule * @returns The type of the module */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/import-type.ts
(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/utils/import-type.ts (1)
src/types.ts (2)
PluginSettings
(113-113)LiteralNodeValue
(198-204)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (13)
src/utils/import-type.ts (13)
9-21
: Well-structured JSDoc for baseModuleThe documentation is clear and includes helpful examples showing how the function handles both scoped and unscoped packages, as well as various path patterns. The return value and parameter descriptions are also well-documented.
31-39
: Clear documentation for isInternalRegexMatchThe JSDoc clearly explains the function's purpose, its relationship to plugin settings, and provides appropriate parameter and return value descriptions.
45-50
: Concise documentation for isAbsoluteThe JSDoc is straightforward and effectively communicates the function's purpose.
55-71
: Comprehensive documentation for isBuiltIn with helpful inline commentThe JSDoc thoroughly explains built-in modules, references the plugin settings, and provides examples. The added inline comment on line 77 also helps clarify the conditional logic in the implementation.
Also applies to: 77-77
114-131
: Clear documentation with examples for isModuleThe JSDoc effectively explains the "loose check" nature of this function and provides excellent examples of what does and doesn't qualify as a module name according to this function's criteria.
144-154
: Good examples in isScoped documentationThe JSDoc includes helpful examples that clarify what qualifies as a scoped module name, which is particularly useful given the specific regex pattern being tested.
165-179
: Thorough documentation for isRelativeToParentThe documentation clearly explains the function's purpose with multiple examples showing both matching and non-matching patterns.
186-199
: Clear isIndex documentation with comprehensive examplesThe JSDoc clearly documents all the patterns that qualify as index files, which is helpful since this relies on a Set of predefined patterns.
204-216
: Good contrast examples in isRelativeToSibling documentationThe JSDoc effectively shows both matching and non-matching examples, which helps clarify the difference between sibling and parent relative paths.
221-230
: Detailed explanation for isExternalPathThe documentation clearly explains what constitutes an external path and references the relevant plugin settings that affect this determination.
256-264
: Concise isInternalPath documentationThe JSDoc succinctly explains what constitutes an internal path, which complements the isExternalPath documentation.
276-285
: Clear examples for isExternalLookingNameThe documentation effectively shows what types of module names are considered "external looking" and matches the implementation that checks for both regular and scoped modules.
9-342
: Overall excellent documentation effortThis PR adds comprehensive JSDoc to almost every function in the file, significantly improving code readability and maintainability. The documentation follows a consistent style, includes clear examples where appropriate, and accurately reflects the function implementations.
🧰 Tools
🪛 Biome (1.9.4)
[error] 82-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
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.
❌ Changes requested. Reviewed everything up to 5de8cdc in 3 minutes and 0 seconds
More details
- Looked at
254
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. src/utils/import-type.ts:120
- Draft comment:
Clarify 'isModule' JSDoc: The examples indicate that 'package-name' is false, but npm package names often include hyphens. Consider updating the description or examples to avoid confusion. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. src/utils/import-type.ts:293
- Draft comment:
Consider renaming the parameter 'path' in 'typeTest' to something like 'modulePath' for clarity, as 'path' is a common module name and could be confused with the Node.js 'path' module. - 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.
3. src/utils/import-type.ts:341
- Draft comment:
The new JSDoc comments across the utility functions are very helpful. Ensure that any future changes to function logic also update the JSDoc to stay consistent. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. src/utils/import-type.ts:55
- Draft comment:
For clarity, consider noting the difference in naming between the exported function isBuiltIn and the imported isBuiltin from 'node:module' in the documentation. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_bb3ZQLZK02GpeZge
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
@JounQin I found some functions inside Also I'm not sure if we could add But these are stuff for another 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.
Great job!
@Shinigami92 Those are excluded as intended, they are all needed to be imported with the extension explicitly, |
As commented in #287 (comment), here are a start of some JSDocs
Important
Add JSDoc comments to functions in
import-type.ts
for improved documentation.import-type.ts
includingbaseModule
,isInternalRegexMatch
,isAbsolute
,isBuiltIn
,isModule
,isScoped
,isRelativeToParent
,isIndex
,isRelativeToSibling
,isExternalPath
,isInternalPath
,isExternalLookingName
,typeTest
, andimportType
.This description was created by
for 5de8cdc. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation