-
-
Notifications
You must be signed in to change notification settings - Fork 325
feat: Add defaultRequest to customRequest #660
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: Add defaultRequest to customRequest #660
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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将上传请求的调用签名扩展为向自定义请求传入额外信息对象 Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Upload 组件
participant AU as AjaxUploader
participant CR as customRequest(fn)
participant DR as defaultRequest
participant S as 服务器
UI->>AU: 选择文件并触发上传
AU->>CR: 调用 customRequest(option, { defaultRequest })
alt customRequest 调用 info.defaultRequest
CR->>DR: info.defaultRequest(option)
DR->>S: 发送上传请求
S-->>DR: 返回响应
DR-->>AU: 通知上传结果(onSuccess/onError)
else customRequest 自行处理上传
CR-->>AU: 直接回调 onSuccess/onError 或 返回含 abort 的对象
end
AU-->>UI: 触发 onStart/onSuccess/onError
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 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 @EmilyyyLiu, 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! 此PR旨在增强 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
这个 PR 的目标是在 customRequest 中提供 defaultRequest,让用户可以在自定义上传逻辑中回退到默认实现,这是一个很好的功能增强。代码实现基本正确,但在 AjaxUploader.tsx 中,确定最终 request 函数的逻辑可以更简洁一些。此外,新添加的测试用例 tests/uploader.spec.tsx 存在一些问题:它采用了一种不寻常的测试模式,即修改传入的 info 对象来注入 mock,并且没有正确地测试功能。测试用例应该被修正以确保功能的正确性和促进正确的 API 用法。我对这两点都提出了具体的修改建议。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #660 +/- ##
=======================================
Coverage 89.03% 89.03%
=======================================
Files 6 6
Lines 301 301
Branches 82 82
=======================================
Hits 268 268
Misses 33 33 ☔ 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 (1)
src/interface.tsx (1)
34-34: 补充 JSDoc 说明新参数约定与兼容性为公共 API 增强可发现性,建议在属性上补充注释,说明“第二个参数
info自本次发版起可用,旧实现可忽略该参数”。应用示例 diff:
- customRequest?: CustomUploadRequestOption; + /** 第二参数 `info` 提供 `defaultRequest`(新能力);旧实现可忽略该参数以保持兼容 */ + customRequest?: CustomUploadRequestOption;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/AjaxUploader.tsx(1 hunks)src/interface.tsx(2 hunks)tests/uploader.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/uploader.spec.tsx
- src/AjaxUploader.tsx
🔇 Additional comments (2)
src/interface.tsx (2)
81-84: 已对齐既往评审意见:info放在第二参数与过往评论“info 放第二个参数,不要和 option 放一起”一致,形参顺序正确。👍
81-84: 结论:将info设为必填以“避免破坏性变更”的担忧不成立仓库检索:src/AjaxUploader.tsx 在调用处会以两个参数传入 customRequest(tests 中有两参实现,docs 示例有一参用法)。TypeScript(含 --strictFunctionTypes)允许参数更少的函数赋值给期望更多参数的函数签名,因此一参实现不会被判为不兼容。建议性改进:可选地按原 diff 添加泛型并将 info 设为可选以提高类型精确度(不会破坏兼容性)。
-export type CustomUploadRequestOption = ( - option: UploadRequestOption, - info: { defaultRequest: (option: UploadRequestOption) => { abort: () => void } | void }, -) => void | { abort: () => void }; +export type CustomUploadRequestOption<T = any> = ( + option: UploadRequestOption<T>, + info?: { defaultRequest: (option: UploadRequestOption<T>) => { abort: () => void } | void }, +) => void | { abort: () => void };位置:src/interface.tsx(类型定义);调用处:src/AjaxUploader.tsx;示例/测试:docs/examples/customRequest.tsx、tests/uploader.spec.tsx。
Likely an incorrect or invalid review comment.
src/AjaxUploader.tsx
Outdated
| const { uid } = origin; | ||
| const request = customRequest || defaultRequest; | ||
|
|
||
| const request = |
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.
为啥不直接去 request 调用的地方改?
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.
request = customRequest || defaultRequest;
如果在调用时候改,有可能 defaultRequest 的参数也加上了{defaultRequest}
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 为 Upload 组件的 customRequest 属性增加了一个新功能,允许在自定义上传逻辑中通过第二个参数 info.defaultRequest 回退使用内置的上传请求。这是一个很好的功能增强,提高了 customRequest 的灵活性。
代码实现方面,逻辑是正确的,并且做了向后兼容。类型定义也已相应更新。
我提出了一些建议:
- 在
tests/uploader.spec.tsx中,修复测试用例里对defaultRequest不正确的调用方式,以确保测试的健壮性和正确性。 - 在
src/AjaxUploader.tsx中,对request变量的赋值逻辑进行少量重构,以提高代码的清晰度。
总体来说,这是一个高质量的 PR,在采纳了建议的修改后即可合并。
src/AjaxUploader.tsx
Outdated
| const request = | ||
| typeof propsCustomRequest === 'function' | ||
| ? args => propsCustomRequest(args, { defaultRequest }) | ||
| : propsCustomRequest || defaultRequest; |
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.
关联issue: ant-design/ant-design#54457
在customRequest 参数中增加 defaultRequest
Summary by CodeRabbit