Skip to content
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

feat(utils): use cursor to add comments, ts type declarations, and vitest test cases to utils functions #3138

Merged
merged 3 commits into from
Mar 21, 2025

Conversation

zzcr
Copy link
Member

@zzcr zzcr commented Mar 17, 2025

使用cursor给utils函数添加注释、ts类型声明和vitest测试用例

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • New Features

    • Added enhanced helper methods for DOM manipulation, event handling, and type validation to enrich the utility toolkit.
  • Documentation

    • Improved inline documentation and type annotations throughout the utilities, offering clearer usage guidelines and better developer support.
  • Tests

    • Expanded the test suite to cover various scenarios, ensuring greater consistency and reliability for the utility functions.

Copy link

coderabbitai bot commented Mar 17, 2025

Walkthrough

This PR enhances the documentation and type annotations for utility functions. In the DOM module, comments are expanded and function signatures updated with explicit types. New functions such as getScrollTop, stopPropagation, preventDefault, getScrollParent, and useScrollParent are added. In the type module, detailed Chinese documentation and explicit return types have been introduced for existing functions, and new functions for prototype and function string representation have been added. A new Vitest test suite validates these type-checking utilities.

Changes

File(s) Change Summary
packages/utils/.../dom/index.ts Updated documentation comments and type annotations for multiple DOM utilities (camelCase, on/off/once, hasClass, addClass, removeClass, getStyle, etc.); added new functions (getScrollTop, stopPropagation, preventDefault, getScrollParent, useScrollParent, and improved isDisplayNone).
packages/utils/.../type/index.ts Expanded documentation (including Chinese) and updated type signatures for functions (toString, hasOwn, isNull, typeOf, isObject, isFunction, isPlainObject, isEmptyObject, isNumber, isNumeric, isDate, isSame, isRegExp, isPromise); added new functions (getProto, fnToString, ObjectFunctionString).
packages/utils/.../type/tests/type.test.ts Introduced a new Vitest unit test file covering various type-checking functions with comprehensive edge cases.

Sequence Diagram(s)

sequenceDiagram
    participant C as Component
    participant H as useScrollParent Hook
    participant SP as getScrollParent Func
    C->>H: Call hook with ref & onMounted callback
    H->>SP: Determine scroll container based on ref and root
    SP-->>H: Return scroll container
    H-->>C: Provide scroll parent reference
Loading

Possibly related PRs

Suggested labels

documentation

Poem

I'm a rabbit with a code-filled heart,
Hopping through docs and playing my part.
I nibble on types with a swift little leap,
Enhancing functions while others sleep.
Joy in each comment, my code sings deep! 🐇💻

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/utils/src/type/__tests__/type.test.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-vue".

(The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-vue@latest --save-dev

The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)
  • We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
    - To enable this feature, set early_access to true under in the settings.
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the enhancement New feature or request (功能增强) label Mar 17, 2025
Copy link

Walkthrough

此PR通过使用cursor工具为utils函数添加了详细的注释和vitest测试用例,增强了代码的可读性和测试覆盖率。主要修改集中在类型判断工具函数的测试和DOM操作工具函数的注释。

Changes

文件 概要
packages/utils/tests/type.test.ts 添加了针对类型判断工具函数的vitest测试用例,涵盖了isNull, typeOf, isObject等多个函数。
packages/utils/src/dom/index.ts 为DOM操作工具函数添加了详细的注释,说明了每个函数的用途、参数和返回值。
packages/utils/src/type/index.ts 为类型判断工具函数添加了详细的注释,说明了每个函数的用途、参数和返回值。

export const once = (el: HTMLElement, event: any, fn: (this: HTMLElement, ev: any) => any) => {
const listener = function () {
const listener = function (this: HTMLElement, ev: any) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of fn.call(this, ev) is correct here, as it ensures the function is called with the correct context and event argument. This change improves the clarity and correctness of the once function.

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Walkthrough

This PR enhances code readability and test coverage by adding detailed comments and vitest test cases to the utils function using the cursor tool. The main modifications focus on the test of type judgment tool functions and the comments of DOM operation tool functions.

Changes

Documents Summary
packages/utils/tests/type.test.ts Added vitest test cases for type judgment tool functions, covering multiple functions such as isNull, typeOf, isObject, etc.
packages/utils/src/dom/index.ts Added detailed comments to the DOM operation tool function, explaining the purpose, parameters and return values ​​of each function.
packages/utils/src/type/index.ts Added detailed comments to the type judgment tool function, explaining the purpose, parameters and return values ​​of each function.

Copy link

[e2e-test-warn]
The component to be tested is missing.

The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug".

Please make sure you've read our contributing guide

Copy link

@coderabbitai coderabbitai bot left a 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 (1)
packages/utils/src/type/index.ts (1)

273-287: Well-documented new isPromise function with examples.

The JSDoc clearly explains that the function checks for both actual Promise objects and Promise-like objects with then/catch methods.

Consider using Number.isFinite instead of isFinite on line 202 for safer type checking:

-export const isNumber = (value: any) => typeof value === 'number' && isFinite(value)
+export const isNumber = (value: any) => typeof value === 'number' && Number.isFinite(value)

The global isFinite performs type coercion, while Number.isFinite is stricter and only returns true for actual number types.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 985f700 and 5e96862.

📒 Files selected for processing (3)
  • packages/utils/__tests__/type.test.ts (1 hunks)
  • packages/utils/src/dom/index.ts (14 hunks)
  • packages/utils/src/type/index.ts (4 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/utils/__tests__/type.test.ts

[error] 124-124: Prefer constants from the standard library.

Unsafe fix: Use Math.PI instead.

(lint/suspicious/noApproximativeNumericConstant)

packages/utils/src/type/index.ts

[error] 20-20: Do not shadow the global "toString" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


[error] 239-239: isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isFinite instead.

(lint/suspicious/noGlobalIsFinite)

🔇 Additional comments (50)
packages/utils/src/dom/index.ts (20)

20-26: Great documentation for camelCase function!

The JSDoc comment clearly describes the function's purpose, parameter, and return value. Including an example helps developers understand exactly how this function works with complex input.


32-38: Good documentation for event binding function.

The JSDoc comment properly explains the purpose and parameters of the on function. You've included the default value for options in both the comment and function signature, which is excellent for documentation consistency.

Also applies to: 39-43


45-51: Clear documentation for event removal function.

The JSDoc for off function mirrors the structure of the on function, making them consistent and easy to understand together.

Also applies to: 52-56


58-63: Improved once function with better documentation.

The JSDoc comment clearly explains the purpose of this one-time event binding function. I notice you've also improved the implementation by using this binding correctly in the listener function.

Also applies to: 64-74


76-82: Well-documented hasClass function with constraints.

Good job clarifying that the function only works with a single class name and explaining the limitation about spaces in the class name parameter.

Also applies to: 83-95


97-102: Clear documentation for addClass function.

The comment correctly explains that multiple classes can be added when separated by spaces, which is important functionality to highlight.

Also applies to: 103-111


113-118: Consistent documentation for removeClass function.

This matches the pattern used for addClass, maintaining a consistent API documentation style.

Also applies to: 119-127


129-135: Detailed explanation of getStyle behavior.

Good documentation explaining how the function prioritizes style lookup (first checking inline styles, then computed styles). This helps developers understand the function's internal logic.

Also applies to: 136-160


162-167: Clear documentation for setStyle function.

The comment explains that the function can accept either a single style property or an object of multiple styles, which is important for users to understand its flexible usage.

Also applies to: 168-184


186-191: Enhanced documentation for isScroll with nested explanation.

The main JSDoc and internal comment together provide a comprehensive explanation of how this function handles directional scrolling detection.

Also applies to: 197-201, 192-212


214-219: Well-documented getScrollContainer function.

The JSDoc clearly explains the function's purpose, parameters, and return value, helping developers understand that it traverses up the DOM tree to find scrollable parents.

Also applies to: 220-240


242-248: Detailed explanation for isInContainer function.

The comment clearly explains that the function requires elements to be fully contained (not just overlapping), which is a subtle but important detail.

Also applies to: 249-274


276-283: Comprehensive documentation for getDomNode function.

The JSDoc provides a detailed breakdown of each property in the returned object, which is very helpful for consumers of this API.

Also applies to: 284-295


297-302: Well-documented new getScrollTop function.

Good explanation of how this function handles the iOS edge case with negative scrollTop values.

Also applies to: 303-307


309-312: Concise documentation for stopPropagation utility.

The JSDoc is simple but effective for this straightforward function.

Also applies to: 313-313


315-319: Clear documentation for preventDefault with conditional logic.

The JSDoc explains the optional isStopPropagation parameter, which allows this function to handle both concerns in one call.

Also applies to: 320-329


334-338: Helpful documentation for isElement helper function.

The JSDoc clearly explains the purpose of this internal utility function.

Also applies to: 339-339


341-346: Well-documented getScrollParent function.

The comment clearly explains the function parameters and return value, helping developers understand how to use it effectively.

Also applies to: 347-361


363-367: Good documentation for new useScrollParent composition function.

The JSDoc effectively explains this higher-order function that integrates with component hooks, which is helpful for those using composable patterns.

Also applies to: 368-378


380-385: Improved documentation for isDisplayNone with recursive behavior.

The JSDoc clearly explains the function's recursive nature, which is important for understanding how it handles nested elements.

Also applies to: 386-404

packages/utils/__tests__/type.test.ts (13)

1-16: Well-structured imports and comprehensive test coverage.

The test file imports all the necessary type utility functions and sets up a good testing structure using Vitest.


17-27: Thorough tests for isNull function.

The tests correctly verify that isNull returns true for null and undefined, and false for other values like empty strings, zero, and objects.


29-44: Comprehensive tests for typeOf function.

The tests cover all possible return types from the function, including edge cases like async functions and error objects.


46-56: Good test cases for isObject function.

The tests correctly verify that only plain objects return true, while arrays, dates, functions, null, and undefined return false.


58-68: Thorough tests for isFunction function.

The tests cover both regular and async functions, using different function declaration syntaxes.


70-84: Excellent tests for isPlainObject with edge cases.

Good job testing with Object.create(null) and custom class instances to ensure the function correctly identifies true plain objects.


86-95: Comprehensive tests for isEmptyObject function.

The tests verify behavior for both objects and arrays, as well as null and undefined.


97-109: Thorough tests for isNumber function.

The tests correctly verify that NaN and Infinity are not considered valid numbers, which is an important edge case.


111-128: Comprehensive tests for isNumeric function.

Excellent coverage of various numeric formats including hexadecimal, scientific notation, and strings that can be parsed as numbers.

🧰 Tools
🪛 Biome (1.9.4)

[error] 124-124: Prefer constants from the standard library.

Unsafe fix: Use Math.PI instead.

(lint/suspicious/noApproximativeNumericConstant)


130-140: Good coverage for isDate function.

The tests correctly distinguish between Date objects and timestamp values or date strings.


142-156: Thorough tests for isSame function.

The tests verify the special handling for NaN equality and correctly check that reference equality is maintained for objects.


157-166: Good tests for isRegExp function.

The tests correctly distinguish between RegExp objects and string representations of regular expressions.


168-186: Thorough tests for isPromise function.

Good job testing both actual Promise instances and Promise-like objects with then/catch methods, as well as objects missing the required methods.

packages/utils/src/type/index.ts (17)

13-17: Good documentation for toString utility.

The comment clearly explains that this is exposing Object.prototype.toString for type checking purposes.


19-23: Clear documentation for hasOwn utility.

The comment explains that this is a reference to the Object.prototype.hasOwnProperty method.

🧰 Tools
🪛 Biome (1.9.4)

[error] 20-20: Do not shadow the global "toString" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


25-29: Concise documentation for getProto utility.

The comment clearly explains that this is a reference to Object.getPrototypeOf.


31-35: Good documentation for fnToString utility.

The comment explains the purpose of this function in determining object constructors.


37-41: Clear documentation for ObjectFunctionString constant.

The comment explains how this is used for identifying plain objects.


43-47: Good documentation for class2type mapping.

The comment explains how this maps toString results to type strings.


60-71: Enhanced documentation for isNull with examples.

The JSDoc now provides clear examples that demonstrate the function's behavior with different inputs.


74-95: Comprehensive documentation for typeOf with examples.

The JSDoc now includes a wide range of examples showing the output for different types of inputs.


99-110: Improved documentation for isObject with examples.

The JSDoc clarifies what constitutes an "object" in this context and provides helpful examples.


113-124: Enhanced documentation for isFunction with examples.

The JSDoc now clearly shows that both regular and async functions are recognized.


127-142: Detailed documentation for isPlainObject with examples.

The JSDoc now includes a more thorough explanation of what constitutes a "plain object" and provides relevant examples.


157-171: Improved documentation for isEmptyObject with examples.

The JSDoc now clearly explains the behavior for different types of inputs.


186-201: Enhanced documentation for isNumber with examples.

The JSDoc now explicitly mentions that NaN and Infinity are not considered valid numbers, which is important to highlight.


204-225: Comprehensive documentation for isNumeric with examples.

The JSDoc includes a wide range of examples showing what values are considered numeric, which is very helpful.


228-238: Improved documentation for isDate with examples.

The JSDoc now includes examples that distinguish between Date objects and other date-related values.


241-256: Detailed documentation for isSame with examples.

The JSDoc explains the special handling for NaN comparison and includes examples that demonstrate different scenarios.


260-270: Clear documentation for isRegExp with examples.

The JSDoc includes examples that distinguish between actual RegExp objects and string representations.

@zzcr zzcr changed the title feat(utils): use cursor to add comments and vitest test cases to utils functions feat(utils): use cursor to add comments, ts type declarations, and vitest test cases to utils functions Mar 17, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
packages/utils/src/type/index.ts (2)

19-23: Consider enhancing the hasOwn function documentation.

The documentation is clear, but the function signature could be improved since it's missing the object on which the property is checked.

- export const hasOwn: (prop: PropertyKey) => boolean = Object.prototype.hasOwnProperty
+ export const hasOwn: <T>(obj: T, prop: PropertyKey) => boolean = Object.prototype.hasOwnProperty.call

This would make the function signature more accurately reflect how it's actually used (with call) throughout the codebase.


203-225: Consider revising isNumeric implementation.

The current implementation can lead to unexpected results in some edge cases. The documentation is very comprehensive with good examples.

- export const isNumeric = (value: any): boolean => value - parseFloat(value) >= 0
+ export const isNumeric = (value: any): boolean => {
+   const num = parseFloat(value);
+   return !isNaN(num) && isFinite(num) && String(num) === String(value).trim();
+ }

This implementation handles more edge cases correctly, like strings with extra whitespace or strings that only partially represent a valid number.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e96862 and 8543985.

📒 Files selected for processing (2)
  • packages/utils/src/dom/index.ts (8 hunks)
  • packages/utils/src/type/index.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/utils/src/dom/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/utils/src/type/index.ts

[error] 17-18: Do not shadow the global "toString" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


[error] 238-238: isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isFinite instead.

(lint/suspicious/noGlobalIsFinite)

🔇 Additional comments (12)
packages/utils/src/type/index.ts (12)

25-29: Good type annotation for getProto function.

The generic type parameter T correctly preserves the type of the input object.


37-41: Ensure ObjectFunctionString is initialized correctly.

The initialization relies on proper implementation of fnToString, which should be fixed as suggested above.


60-72: Great documentation for the isNull function!

The JSDoc comment is comprehensive with clear examples that demonstrate the function's behavior.


74-96: Well-structured documentation for typeOf function.

The examples cover a wide range of input types, which is very helpful for users.

Note that if you rename the toString function as suggested earlier, you'll need to update its usage here:

- export const typeOf = (obj: any): string => (isNull(obj) ? String(obj) : class2type[toString.call(obj)] || 'object')
+ export const typeOf = (obj: any): string => (isNull(obj) ? String(obj) : class2type[objToString.call(obj)] || 'object')

98-110: Clear documentation for isObject function.

The examples effectively illustrate what is considered an object and what isn't.


112-124: Good improvement to isFunction documentation.

The examples cover different function syntaxes including async functions and arrow functions.


126-154: Comprehensive documentation and improved implementation for isPlainObject.

The examples and the additional explanation about checking for objects with null prototype are helpful. The type annotation for proto is also a good addition.


156-183: Improved documentation for isEmptyObject with clear examples.

The implementation now has a properly typed type variable which is good.


227-238: Good documentation for isDate function.

The examples clearly differentiate between actual Date objects and other date-related values.

🧰 Tools
🪛 Biome (1.9.4)

[error] 238-238: isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isFinite instead.

(lint/suspicious/noGlobalIsFinite)


240-257: Great documentation enhancement for isSame function.

The explanation about special handling for NaN comparison is particularly helpful.


259-270: Clear documentation for isRegExp function.

The examples effectively distinguish between actual RegExp objects and string representations.


272-286: Comprehensive documentation for isPromise function.

The explanation about "class Promise" objects with then/catch methods is particularly useful.

Comment on lines +13 to +17
/**
* 获取对象的字符串表示形式
* 用于类型判断,等同于 Object.prototype.toString
*/
export const toString: () => string = Object.prototype.toString
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Good addition of JSDoc documentation for the toString function.

The documentation clearly explains the purpose of this exported function. However, there's an issue with shadowing the global toString property that could lead to confusion.

Consider renaming this to avoid shadowing the global property:

- export const toString: () => string = Object.prototype.toString
+ export const objToString: () => string = Object.prototype.toString

Also, update any references to this function throughout the codebase.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* 获取对象的字符串表示形式
* 用于类型判断,等同于 Object.prototype.toString
*/
export const toString: () => string = Object.prototype.toString
/**
* 获取对象的字符串表示形式
* 用于类型判断,等同于 Object.prototype.toString
*/
export const objToString: () => string = Object.prototype.toString

Comment on lines +31 to +35
/**
* 获取函数的字符串表示
* 用于判断对象构造函数
*/
const fnToString: () => string = hasOwn.toString
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix incorrect function signature for fnToString.

The current implementation suggests that fnToString is a direct reference to hasOwn.toString, but the way you're using it later (with .call) indicates it should be properly typed.

- const fnToString: () => string = hasOwn.toString
+ const fnToString: Function["toString"] = Function.prototype.toString
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* 获取函数的字符串表示
* 用于判断对象构造函数
*/
const fnToString: () => string = hasOwn.toString
/**
* 获取函数的字符串表示
* 用于判断对象构造函数
*/
const fnToString: Function["toString"] = Function.prototype.toString

Comment on lines 185 to +201
/**
* 判断对象是否为数字类型。
* 判断对象是否为数字类型(有限数字)
*
* 注意:NaN和Infinity不被视为有效数字
*
* @param {any} value - 要检查的值
* @returns {boolean} - 如果是有效数字则返回true,否则返回false
*
* isNumber(369) // true
* @example
* isNumber(369) // true
* isNumber(0) // true
* isNumber(-1.5) // true
* isNumber(NaN) // false
* isNumber(Infinity) // false
* isNumber('123') // false
*/
export const isNumber = (value: any) => typeof value === 'number' && isFinite(value)
export const isNumber = (value: any): boolean => typeof value === 'number' && isFinite(value)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Security issue with isNumber implementation.

The function correctly uses isFinite to check for valid numbers, but according to static analysis, there's a potential security concern.

- export const isNumber = (value: any): boolean => typeof value === 'number' && isFinite(value)
+ export const isNumber = (value: any): boolean => typeof value === 'number' && Number.isFinite(value)

The global isFinite() attempts type coercion which could lead to unexpected results, while Number.isFinite() doesn't perform type coercion and is safer.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* 判断对象是否为数字类型。
* 判断对象是否为数字类型(有限数字)
*
* 注意:NaN和Infinity不被视为有效数字
*
* @param {any} value - 要检查的值
* @returns {boolean} - 如果是有效数字则返回true,否则返回false
*
* isNumber(369) // true
* @example
* isNumber(369) // true
* isNumber(0) // true
* isNumber(-1.5) // true
* isNumber(NaN) // false
* isNumber(Infinity) // false
* isNumber('123') // false
*/
export const isNumber = (value: any) => typeof value === 'number' && isFinite(value)
export const isNumber = (value: any): boolean => typeof value === 'number' && isFinite(value)
/**
* 判断对象是否为数字类型(有限数字)
*
* 注意:NaN和Infinity不被视为有效数字
*
* @param {any} value - 要检查的值
* @returns {boolean} - 如果是有效数字则返回true,否则返回false
*
* @example
* isNumber(369) // true
* isNumber(0) // true
* isNumber(-1.5) // true
* isNumber(NaN) // false
* isNumber(Infinity) // false
* isNumber('123') // false
*/
export const isNumber = (value: any): boolean =>
typeof value === 'number' && Number.isFinite(value)

Copy link

@coderabbitai coderabbitai bot left a 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 (3)
packages/utils/src/type/__tests__/type.test.ts (3)

48-49: Remove duplicate test case

There are two identical test cases for isObject({}).

 expect(isObject({})).toBe(true)
-expect(isObject({})).toBe(true)

72-73: Remove duplicate test case

There are two identical test cases for isPlainObject({}).

 expect(isPlainObject({})).toBe(true)
-expect(isPlainObject({})).toBe(true)

118-118: Use Math.PI instead of approximation

Replace the approximation of Pi with the standard library constant.

-      expect(isNumeric(3.1415)).toBe(true)
+      expect(isNumeric(Math.PI)).toBe(true)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8543985 and b044569.

📒 Files selected for processing (1)
  • packages/utils/src/type/__tests__/type.test.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/utils/src/type/__tests__/type.test.ts

[error] 124-124: Prefer constants from the standard library.

Unsafe fix: Use Math.PI instead.

(lint/suspicious/noApproximativeNumericConstant)

🔇 Additional comments (12)
packages/utils/src/type/__tests__/type.test.ts (12)

1-16: Well-structured imports and organization!

Good job setting up the test file with proper imports from Vitest and importing all the utility functions to be tested. This organization makes the test file clean and easy to follow.


17-27: Good test coverage for isNull function!

Your tests correctly verify that isNull returns true for both null and undefined, and false for other values like empty strings, zero, false, and objects. This comprehensive approach ensures the function behaves as expected.


29-44: Thorough typeOf function testing!

The tests cover all major JavaScript types including primitive values, functions (both regular and async), arrays, dates, errors, regular expressions, and objects. Particularly good job testing the async function case which ensures the function correctly identifies this specific type.


58-68: Excellent function type tests!

Good job testing various function types including regular functions, async functions, arrow functions, and async arrow functions. This comprehensive approach ensures the utility correctly identifies all forms of JavaScript functions.


80-82: Great edge case with custom class!

Good job testing isPlainObject with a custom class instance. This is an important distinction for this function since it should differentiate between plain objects and instances of custom classes.


86-95: Comprehensive empty object testing!

Your tests for isEmptyObject correctly verify that empty objects and arrays return true, while objects and arrays with values return false. The handling of edge cases like null and undefined is also properly tested.


97-109: Thorough number type testing!

Good coverage of number type testing including special cases like NaN and Infinity. The comments on lines 103-104 are helpful for explaining why these values aren't considered valid numbers in this context.


130-140: Good Date object validation tests!

Your tests correctly distinguish between actual Date objects and other date-like values such as timestamps and date strings. This ensures the function works as intended for all edge cases.


142-155: Excellent equality comparison tests!

Good job testing isSame with various primitive values and the special handling for NaN. The tests correctly verify that objects and arrays with the same content but different references are not considered the same, and values of different types are not equal even if they have similar representations.


157-178: Well-structured RegExp tests!

Great use of it.each to organize multiple test cases for regular expressions. This approach makes the tests more readable and maintainable. The test cases cover various regex patterns and correctly identify non-regex values.


180-197: Complete Promise detection tests!

Good job testing both actual Promise objects and Promise-like objects with then and catch methods. The test on line 193 correctly verifies that objects missing the catch method are not considered Promises, which is an important distinction.


1-198: Overall excellent test coverage!

This test file provides comprehensive coverage for all the type utility functions. The tests are well-organized, cover edge cases, and include helpful comments. This will ensure the reliability of these critical utility functions.

A few minor suggestions:

  1. Remove the duplicate test cases for isObject and isPlainObject
  2. Use Math.PI instead of the approximation 3.1415

Otherwise, this is a great addition to the codebase!

🧰 Tools
🪛 Biome (1.9.4)

[error] 124-124: Prefer constants from the standard library.

Unsafe fix: Use Math.PI instead.

(lint/suspicious/noApproximativeNumericConstant)

@zzcr zzcr marked this pull request as draft March 19, 2025 02:35
@zzcr zzcr marked this pull request as ready for review March 20, 2025 12:26
@github-actions github-actions bot added the bug Something isn't working label Mar 21, 2025
@kagol kagol merged commit dedc9f1 into dev Mar 21, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request (功能增强)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants