-
Notifications
You must be signed in to change notification settings - Fork 197
feat: support getId #724
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: support getId #724
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough新增公开函数 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 分钟 Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @zombieJ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #724 +/- ##
==========================================
+ Coverage 85.92% 86.34% +0.42%
==========================================
Files 38 38
Lines 1016 1040 +24
Branches 380 371 -9
==========================================
+ Hits 873 898 +25
Misses 141 141
+ Partials 2 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.
Code Review
这个 PR 添加了一个工具函数 getId 用于生成合法的 HTML ID,并为其添加了相应的测试。这是一个实用的功能。然而,实现中存在一个严重的 bug:用于清理 key 的正则表达式不正确,导致其行为与预期和文档不符。我已经提供了一个修复建议。
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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: 1
🤖 Fix all issues with AI agents
In `@src/hooks/useId.ts`:
- Around line 28-33: Change getId to accept React-compatible key types (e.g.,
string | number | bigint | null | undefined) instead of only string, then
normalize the incoming key to a safe string before sanitization: treat
null/undefined as an empty string, convert numbers and bigints with toString(),
and then apply the existing regex sanitization; update the function signature
(getId) and any callers/types accordingly so it no longer assumes a string
input.
🧹 Nitpick comments (1)
src/hooks/useId.ts (1)
29-33: 注意非法字符替换可能导致 ID 冲突
'a b'与'a-b'会被规范化为同一个 ID。若该 ID 用于 DOM/ARIA 关联,可能出现重复冲突。建议至少在文档中说明唯一性要求,或在发生替换时追加短 hash/转义以保持唯一性。
| export function getId(prefix: string, key: string): string { | ||
| // Valid id characters: letters, digits, hyphen, underscore, colon, period | ||
| // Replace all invalid characters (including spaces) with hyphens to preserve length | ||
| const sanitizedKey = key.replace(/[^a-zA-Z0-9_.:-]/g, '-'); | ||
|
|
||
| return `${prefix}-${sanitizedKey}`; |
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.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual file
cat -n src/hooks/useId.tsRepository: react-component/util
Length of output: 2688
🏁 Script executed:
# Search for how getId is being called to understand usage patterns
rg "getId" --type ts --type tsx -B 2 -A 2Repository: react-component/util
Length of output: 91
🌐 Web query:
React.Key type definition React 18 - what types are included in React.Key?
💡 Result:
In React 18’s TypeScript types (@types/react), React.Key is defined as:
string | number | bigint(app.unpkg.com)
And the JSX key attribute is typed as:
key?: React.Key | null | undefined(app.unpkg.com)
Citations:
- 1: https://app.unpkg.com/%40types/react%4018.2.37/files/index.d.ts
- 2: https://app.unpkg.com/%40types/react%4018.2.37/files/index.d.ts
🏁 Script executed:
# Search for getId function calls to understand usage patterns
rg "getId\(" --type ts -B 2 -A 2Repository: react-component/util
Length of output: 1160
🏁 Script executed:
# Also check if this is exported/used elsewhere
rg "from.*useId|import.*getId" --type ts --type tsxRepository: react-component/util
Length of output: 91
🏁 Script executed:
# Check if there are any tests or examples showing how getId should be used
fd -e test.ts -e spec.ts -e example.ts | head -20Repository: react-component/util
Length of output: 392
React 元素 key 类型与入参类型不匹配,存在运行时风险
JSDoc 注释声称该函数处理"React 元素的 key",但 React.Key 的实际类型为 string | number | bigint(JSX 上下文中还可以是 null | undefined)。当前函数签名仅接受 string 参数,若调用者传入数字或其他类型的 React element key,将在运行时抛错。建议扩大入参类型并添加类型安全的规范化处理。
🧩 修复建议
-export function getId(prefix: string, key: string): string {
+export function getId(prefix: string, key: React.Key): string {
+ const normalizedKey = key == null ? '' : String(key);
// Valid id characters: letters, digits, hyphen, underscore, colon, period
// Replace all invalid characters (including spaces) with hyphens to preserve length
- const sanitizedKey = key.replace(/[^a-zA-Z0-9_.:-]/g, '-');
+ const sanitizedKey = normalizedKey.replace(/[^a-zA-Z0-9_.:-]/g, '-');
return `${prefix}-${sanitizedKey}`;
}📝 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.
| export function getId(prefix: string, key: string): string { | |
| // Valid id characters: letters, digits, hyphen, underscore, colon, period | |
| // Replace all invalid characters (including spaces) with hyphens to preserve length | |
| const sanitizedKey = key.replace(/[^a-zA-Z0-9_.:-]/g, '-'); | |
| return `${prefix}-${sanitizedKey}`; | |
| export function getId(prefix: string, key: React.Key): string { | |
| const normalizedKey = key == null ? '' : String(key); | |
| // Valid id characters: letters, digits, hyphen, underscore, colon, period | |
| // Replace all invalid characters (including spaces) with hyphens to preserve length | |
| const sanitizedKey = normalizedKey.replace(/[^a-zA-Z0-9_.:-]/g, '-'); | |
| return `${prefix}-${sanitizedKey}`; | |
| } |
🤖 Prompt for AI Agents
In `@src/hooks/useId.ts` around lines 28 - 33, Change getId to accept
React-compatible key types (e.g., string | number | bigint | null | undefined)
instead of only string, then normalize the incoming key to a safe string before
sanitization: treat null/undefined as an empty string, convert numbers and
bigints with toString(), and then apply the existing regex sanitization; update
the function signature (getId) and any callers/types accordingly so it no longer
assumes a string input.
做一个格式化 id function 到 rc-tree 里使用:
Summary by CodeRabbit
新增功能
测试
✏️ Tip: You can customize this high-level summary in your review settings.