-
-
Notifications
You must be signed in to change notification settings - Fork 325
fix: uid conflict when multiple upload components paste at same time #686
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
base: master
Are you sure you want to change the base?
Conversation
|
@arksia is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough在粘贴/拖放文件处理路径中,AjaxUploader 将来自 dataTransfer.files 的每个文件显式重建为新的 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 分钟
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 @arksia, 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! 此拉取请求修复了一个关键错误,该错误影响了当多个组件同时处理粘贴文件时的文件上传功能。核心问题源于共享的文件引用导致 Highlights
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
|
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
你好,感谢你的贡献!这个修复通过为每个上传组件克隆文件对象,巧妙地解决了多个上传组件同时处理粘贴事件时可能出现的 uid 冲突问题。这是一个很棒的解决方案。我还添加了一些建议来改进代码和新添加的测试用例,使其更加健壮和简洁。请查看我的评论。
src/AjaxUploader.tsx
Outdated
| let files: File[] = [...(dataTransfer.files || [])].map((file: File) => { | ||
| return new File([file], file.name, { | ||
| type: file.type, | ||
| lastModified: file.lastModified, | ||
| }); | ||
| }); |
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.
为了代码更简洁,这里的 map 函数可以简化为单行表达式,使用隐式返回。
| let files: File[] = [...(dataTransfer.files || [])].map((file: File) => { | |
| return new File([file], file.name, { | |
| type: file.type, | |
| lastModified: file.lastModified, | |
| }); | |
| }); | |
| let files: File[] = [...(dataTransfer.files || [])].map(file => ( | |
| new File([file], file.name, { | |
| type: file.type, | |
| lastModified: file.lastModified, | |
| }) | |
| )); |
| await sleep(100); | ||
| requests[0].respond(200, {}, `["","${files[0].name}"]`); |
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.
这个测试用例很好地覆盖了并发粘贴的场景。不过,它可以更健壮一些。目前它只完成了一个上传请求,并且没有验证两个组件生成的 uid 是唯一的。
我建议更新测试以:
- 断言创建了两个独立的请求。
- 断言
uid是不同的。 - 完成两个上传请求,以确保两个组件的生命周期回调都得到测试。
await sleep(100);
expect(requests).toHaveLength(2);
expect(uid1).toBeDefined();
expect(uid2).toBeDefined();
expect(uid1).not.toEqual(uid2);
// 完成两个上传
requests[0].respond(200, {}, `["","${files[0].name}"]`);
requests[1].respond(200, {}, `["","${files[0].name}"]`);
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #686 +/- ##
==========================================
+ Coverage 89.58% 89.62% +0.03%
==========================================
Files 6 6
Lines 317 318 +1
Branches 90 89 -1
==========================================
+ Hits 284 285 +1
Misses 33 33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
看下 gemini 的 review 建议 |
已根据 review 调整补充 |
修复多个上传组件同时粘贴上传会覆写 dataTransfer 中文件的 uid,导致文件状态更新异常的问题。

图中两个文件上传都失败了,但是第一个文件始终处于 uploading 状态
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.