fix: Cloud files throwing error on upload #685
Conversation
|
@divyeshagrawal 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该 PR 在 AjaxUploader 中新增异步缓存步骤：读取传入的 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 @divyeshagrawal, 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 addresses and resolves an issue where files selected from cloud storage were failing to upload. The core of the fix lies in introducing a new file caching mechanism that processes all incoming files by reading their binary data and creating fresh 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #685 +/- ##
==========================================
- Coverage 89.58% 89.53% -0.06%
==========================================
Files 6 6
Lines 317 325 +8
Branches 90 95 +5
==========================================
+ Hits 284 291 +7
- Misses 33 34 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a fix for an issue where files from cloud storage would fail to upload. The approach of reading the file into an ArrayBuffer to create a local copy before uploading is a good solution. The main logic change in src/AjaxUploader.tsx is sound. I've made a suggestion to improve type safety in the new cacheFiles function.
The test file tests/uploader.spec.tsx has been updated extensively. However, I've found a recurring issue with the mock implementations for file.arrayBuffer() which are incorrect and could lead to test failures. I've provided a comment with a suggested fix for this pattern.
Overall, this is a good fix, and with the suggested changes, it will be even better.
tests/uploader.spec.tsx
Outdated
| function Item(name) { | ||
| this.name = name; | ||
| this.toString = () => this.name; | ||
| this.arrayBuffer = () => this; |
There was a problem hiding this comment.
The mock implementation of arrayBuffer is incorrect. It should return a Promise that resolves to an ArrayBuffer, but here it returns the object instance itself (this). The await file.arrayBuffer() in cacheFiles will resolve to the file-like object, and then new File([buffer], ...) will likely fail as the object is not a valid BlobPart.
This issue is repeated for many mock file objects in this test file. A more correct implementation would be this.arrayBuffer = () => Promise.resolve(new ArrayBuffer());.
| this.arrayBuffer = () => this; | |
| this.arrayBuffer = () => Promise.resolve(new ArrayBuffer()); |
src/AjaxUploader.tsx
Outdated
| cacheFiles = async (files: File[]): Promise<RcFile[]> => { | ||
| if (files?.length) { | ||
| const filesArray = [...files]; | ||
|
|
||
| const cachedFiles = await Promise.all( | ||
| filesArray.map(async file => { | ||
| const buffer = await file.arrayBuffer(); | ||
| return new File([buffer], file.name, { | ||
| type: file.type, | ||
| lastModified: file.lastModified, | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| return cachedFiles as RcFile[]; | ||
| } | ||
|
|
||
| return []; | ||
| }; | ||
|
|
||
| uploadFiles = async (files: File[]) => { | ||
| const originFiles = await this.cacheFiles(files); |
There was a problem hiding this comment.
The cacheFiles function is declared to return a Promise<RcFile[]>, but the File objects created inside do not have a uid property, so they are not RcFiles. Casting cachedFiles as RcFile[] on line 187 is type-unsafe. The function should promise File[] instead. The casting can be moved to uploadFiles.
Also, filesArray on line 175 is a redundant copy of the files array and can be removed.
cacheFiles = async (files: File[]): Promise<File[]> => {
if (files?.length) {
const cachedFiles = await Promise.all(
files.map(async file => {
const buffer = await file.arrayBuffer();
return new File([buffer], file.name, {
type: file.type,
lastModified: file.lastModified,
});
}),
);
return cachedFiles;
}
return [];
};
uploadFiles = async (files: File[]) => {
const originFiles = (await this.cacheFiles(files)) as RcFile[];
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/AjaxUploader.tsx (1)
193-213:uploadFiles变为 async 后建议加一层错误兜底，避免未处理的 Promise 拒绝现在
uploadFiles是async，如果cacheFiles(files)内部因为file.arrayBuffer()读文件失败而 reject，那么整个uploadFiles的 Promise 也会 reject。但调用方（onChange/onDataTransferFiles）只是直接调用this.uploadFiles(...)，没有await或.catch，在浏览器或测试环境中容易出现 “Unhandled promise rejection” 的全局警告，而且不会触发任何用户可见的错误回调。可以在函数内部包一层
try/catch来显式处理这些异常，例如：- uploadFiles = async (files: File[]) => { - const originFiles = await this.cacheFiles(files); - const postFiles = originFiles.map((file: RcFile & { uid?: string }) => { - // eslint-disable-next-line no-param-reassign - file.uid = getUid(); - return this.processFile(file, originFiles); - }); - - // Batch upload files - Promise.all(postFiles).then(fileList => { - const { onBatchStart } = this.props; - - onBatchStart?.(fileList.map(({ origin, parsedFile }) => ({ file: origin, parsedFile }))); - - fileList - .filter(file => file.parsedFile !== null) - .forEach(file => { - this.post(file); - }); - }); - }; + uploadFiles = async (files: File[]) => { + try { + const originFiles = await this.cacheFiles(files); + const postFiles = originFiles.map((file: RcFile & { uid?: string }) => { + // eslint-disable-next-line no-param-reassign + file.uid = getUid(); + return this.processFile(file, originFiles); + }); + + // Batch upload files + const fileList = await Promise.all(postFiles); + const { onBatchStart } = this.props; + + onBatchStart?.(fileList.map(({ origin, parsedFile }) => ({ file: origin, parsedFile }))); + + fileList + .filter(file => file.parsedFile !== null) + .forEach(file => { + this.post(file); + }); + } catch (error) { + // 这里可以按需调用 onError 或至少 console.error，避免未处理的 Promise 拒绝 + } + };这样既保持了原有的批量语义，又能在极端 I/O 失败场景下更可控地处理错误。
🧹 Nitpick comments (7)
src/AjaxUploader.tsx (1)
173-191:cacheFiles异步克隆 File 的实现基本合理这里按 issue 建议，对每个传入的
File执行await file.arrayBuffer()再用同名、同type、lastModified构造新的File，再在后续链路中统一按RcFile处理，能够解决云端占位文件在后续步骤直接进入 error 状态的问题，整体逻辑是正确的。唯一需要注意的是，这会对所有文件（包括本地普通文件）做一次完整读取和重建，对大文件或多文件上传会增加一次内存占用和 I/O 时间。如果后续在性能或内存上有压力，可以考虑再引入一定的启用条件（例如只在检测到特定环境或配置时才走克隆路径），当前版本先保持简单实现也可以接受。
tests/uploader.spec.tsx (6)
10-14: 给Item增加arrayBuffer以兼容新上传管线是合适的
Item现在实现了arrayBuffer，并返回this，配合构造目录上传的虚拟文件系统节点，可以顺利通过cacheFiles中的await file.arrayBuffer()和后续new File([buffer], ...)流程，满足拖拽目录场景的测试需要。如果希望更贴近真实 API，可以考虑让它返回一个 Promise（例如
() => Promise.resolve(this)或Promise.resolve(new ArrayBuffer(0))），但目前实现对测试行为没有实际影响，属于风格层面。
89-95: Blob.prototype.arrayBuffer polyfill 中的this使用略显迷惑，可简化这里在
beforeAll里为缺失Blob.prototype.arrayBuffer的环境做了 polyfill，是必要的。不过当前实现使用了箭头函数：Blob.prototype.arrayBuffer = () => { return new Promise(resolve => resolve(this ?? new ArrayBuffer())); };箭头函数的
this是词法绑定的，不会指向实际的 Blob 实例，因此这里的this通常不会是调用者，而是undefined。最终效果等价于始终返回一个新的ArrayBuffer，对测试来说是可用的，但读起来比较误导。可以考虑改成更直观的版本，例如：
- Blob.prototype.arrayBuffer = () => { - return new Promise(resolve => resolve(this ?? new ArrayBuffer())); - }; + Blob.prototype.arrayBuffer = function () { + // 对测试来说，只要能返回一个可被 File 接受的 BlobPart 即可 + return Promise.resolve(new ArrayBuffer(0)); + };这样避免对
this的误用，也更符合常见 polyfill 写法。
207-405: 为各类上传场景的 mock 文件补充arrayBuffer是必要的，小建议统一写法并减少超时依赖这一段多个用例（如 “upload success/error”、“drag to upload”、“drag unaccepted type files”、“paste to upload”等）都给 mock 文件对象增加了
arrayBuffer实现，以适配AjaxUploader.cacheFiles的新路径，这是必须的，否则这些对象不具备 File/Blob 的 arrayBuffer 接口。小点建议：
目前有两类写法：
- 方法形式：
arrayBuffer() { return this; }- 属性箭头函数：
arrayBuffer: () => { return this; }对于后者，由于箭头函数的
this是词法作用域，不会指向该对象本身，因此this实际上不是文件对象。不过在当前测试中，await file.arrayBuffer()返回什么并不重要（new File([buffer], ...)依然可以接受），所以不会造成失败。如果想语义更一致，建议统一为方法形式。
drag files with multiple false中的延时由 100ms 增加到 150ms 以适配新的异步流程，这能缓解 race 问题，但依赖固定setTimeout值在 CI 上仍可能偏脆弱。长期可以考虑用已有的sleep辅助函数或 RTL 的waitFor来等待断言条件，而非硬编码时间窗口。整体而言，这里的改动能很好覆盖新异步缓存路径下的各种上传入口场景。
Also applies to: 406-461
594-645: 目录上传相关用例中为虚拟目录结构补充arrayBuffer是合理的在 “directory uploader” 这组用例中，代表目录树的
files对象同样增加了arrayBuffer实现，用于：
- 目录选择场景
beforeUpload should run after all children files are parsed- 不接受类型、不触发
onStart的场景- 异步
readEntries正常/报错场景- 通过选择目录触发的 accept 过滤等
这些虚拟节点会通过
makeFileSystemEntry/makeFileSystemEntryAsync走到traverseFileTree，最终进入cacheFiles，因此补充arrayBuffer能保证与真实 File 对象一致地通过新上传前处理流程。和前面的建议类似，这些
arrayBuffer方法只需返回一个可被File构造接受的值即可；如果后续修改，可以考虑抽一个小工厂函数来减少样板代码。Also applies to: 646-672, 690-731, 733-791, 793-810, 812-865
873-943:accept分组中为各个 mock 文件补充arrayBuffer，保持与过滤逻辑解耦在 “accept” 分组里，所有用于测试不同
accept组合（扩展名、MIME、*/*/*、非法类型等）的 mock 文件对象都增加了arrayBuffer属性，以便它们可以顺利通过cacheFiles的异步克隆。这里的主要验证仍然是：
- 文件名和
type是否按accept/attr-accept规则被过滤；- 当
accept非法时会走 warning 但放行。
arrayBuffer在这些用例中只是让对象看起来更像 File/Blob，实际测试断言没有依赖它的具体返回值，当前实现足够且不会影响原有行为。Also applies to: 944-1014, 1016-1065, 1067-1131, 1134-1159
1443-1448: 在beforeUpload用例中结合act和arrayBuffer，正确覆盖动态 action 行为
beforeUpload中通过await act(() => { setAction('bamboo'); });触发并刷新父组件状态，再sleep(100)，确保在processFile读取this.props.action时已经拿到更新后的'bamboo'，从而使请求 URL 正确变为'bamboo'。这是对 “beforeUpload 可同步更新 action” 场景的可靠覆盖。从风格上看，也可以写成：
await act(async () => { setAction('bamboo'); });更符合 React 18 对异步
act的推荐，但当前写法在测试中也是可工作的。mock 文件增加
arrayBuffer使其能通过cacheFiles，与其他用例保持一致。整体来看，这个用例很好地保证了在新异步上传管线下，
beforeUpload里动态修改action仍然生效。Also applies to: 1464-1466
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/AjaxUploader.tsx(1 hunks)tests/uploader.spec.tsx(34 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-09T06:22:00.335Z
Learnt from: zombieJ
Repo: react-component/upload PR: 543
File: src/AjaxUploader.tsx:116-116
Timestamp: 2025-04-09T06:22:00.335Z
Learning: 在 react-component/upload 项目中，黏贴上传功能应该是可配置的，只有当启用此功能时才应添加 paste 事件监听器，而不是无条件添加。同时，事件监听器绑定应使用预先绑定的方法而非直接使用 .bind(this) 以避免内存泄漏。
Applied to files:
tests/uploader.spec.tsx
🧬 Code graph analysis (2)
tests/uploader.spec.tsx (1)
src/attr-accept.ts (1)
file(4-53)
src/AjaxUploader.tsx (1)
src/interface.tsx (1)
RcFile(88-90)
🔇 Additional comments (4)
tests/uploader.spec.tsx (4)
2-2: 引入act以支持异步用例是合理的这里把
act从@testing-library/react一并引入，用于后面 “dynamic change action in beforeUpload should work” 的异步状态更新用例，符合 React Testing Library 的推荐用法，没有问题。
1191-1208: AcceptConfig 用例中补充arrayBuffer，验证新 filter 行为在 “AcceptConfig” 分组中，所有传入的
files对象同样增加了arrayBuffer，以通过cacheFiles。这些用例主要在验证：
- 仅
format生效时的行为；filter: 'native'时跳过 format 检查；- 自定义
filter函数时是否以自定义逻辑为准；- 使用 MIME 通配符
image/*的场景。引入
arrayBuffer后，不会改变这些过滤逻辑的本质，只是让 mock 更接近真实的RcFile。整体实现与新filterFile/attr-accept的配合是合理的。Also applies to: 1211-1228, 1231-1252, 1254-1274
1287-1328:transform file before request用例对新管线的覆盖到位这里的用例在
transformFile返回null时，确保：
- 上传仍然成功；
onSuccess收到的file仍然带有uid和正确的name。通过给 mock 文件增加
arrayBuffer，该用例可以完整走过cacheFiles→processFile（parsedData为null，最终退回原始file） →post的链路，覆盖了 transform 结果为空时的新行为，设计合理。
1331-1340:onBatchStart用例更新与新的 ParsedFileInfo 结构一致
顶部的
batchEventFiles现在断言file至少包含正确的name、type，与通过cacheFiles克隆出来的origin一致，这很好地避免了过度依赖对象引用。“beforeUpload but one is deny” 用例中新增了对
parsedFile的断言：当某个文件被beforeUpload返回false拒绝时，对应项的parsedFile为null，其余文件parsedFile仍是带有name、type的对象。这与processFile在transformedFile === false时返回{ parsedFile: null, action: null, data: null }的实现完全一致，也验证了uploadFiles中过滤parsedFile !== null再实际发请求的逻辑。这些断言很好地锁定了新批量处理行为的约定。
Also applies to: 1395-1411
|
/gemini review |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/AjaxUploader.tsx (2)
191-197: 类型不安全的转换。在第 192 行，将
cacheFiles的结果立即转换为RcFile[]，但返回的 File 对象此时尚不包含uid属性。uid直到第 195 行才被添加。这种转换在类型上是不安全的。建议的修复方案（根据之前的审查意见）：
uploadFiles = async (files: File[]) => { - const originFiles = (await this.cacheFiles(files)) as RcFile[]; + const cachedFiles = await this.cacheFiles(files); - const postFiles = originFiles.map((file: RcFile & { uid?: string }) => { + const postFiles = cachedFiles.map((file: File) => { // eslint-disable-next-line no-param-reassign - file.uid = getUid(); - return this.processFile(file, originFiles); + const rcFile = file as RcFile; + rcFile.uid = getUid(); + return this.processFile(rcFile, cachedFiles as RcFile[]); });
173-189: 添加返回类型注解并处理潜在错误。该方法缺少显式返回类型注解。此外，
arrayBuffer()调用可能因多种原因失败（网络错误、权限问题、云文件不可用等），但当前实现没有错误处理。对于云文件场景，这可能导致整个上传批次静默失败。应用此差异添加返回类型并包装错误处理：
- cacheFiles = async (files: File[]) => { + cacheFiles = async (files: File[]): Promise<File[]> => { if (files?.length) { const cachedFiles = await Promise.all( files.map(async file => { - const buffer = await file.arrayBuffer(); - return new File([buffer], file.name, { - type: file.type, - lastModified: file.lastModified, - }); + try { + const buffer = await file.arrayBuffer(); + return new File([buffer], file.name, { + type: file.type, + lastModified: file.lastModified, + }); + } catch (error) { + // For cloud files that fail to load, return original file + console.error(`Failed to cache file ${file.name}:`, error); + return file; + } }), ); return cachedFiles; } return []; };注意： 根据之前的审查意见，此方法应返回
Promise<File[]>而非Promise<RcFile[]>，因为创建的 File 对象不包含uid属性。类型转换应在调用处进行。tests/uploader.spec.tsx (3)
10-14: Mock 的arrayBuffer实现不正确。
Item构造函数中的arrayBuffer方法返回Promise.resolve(this)，即返回 Item 对象本身，而不是ArrayBuffer。这与生产代码中cacheFiles的期望不匹配，后者尝试使用new File([buffer], ...)来处理结果，期望buffer是有效的BlobPart（ArrayBuffer）。应用此差异修复（根据之前的审查意见）：
function Item(name) { this.name = name; this.toString = () => this.name; - this.arrayBuffer = () => Promise.resolve(this); + this.arrayBuffer = () => Promise.resolve(new ArrayBuffer(0)); }注意： 此问题在整个测试文件的多个 mock 文件对象中重复出现（第 215-217、249-251、280-282、314-316、382-384、414-416 行等）。所有这些实例都应该以类似方式修复。
215-217: 多个测试文件 mock 中的arrayBuffer实现不正确。在多个测试用例中，文件 mock 对象的
arrayBuffer()方法返回Promise.resolve(this)，这与预期的ArrayBuffer不符。这些不正确的实现会导致测试无法准确反映生产环境中的行为，其中new File([buffer], ...)期望接收实际的ArrayBuffer作为BlobPart。这些行都应该更新为：
arrayBuffer() { - return Promise.resolve(this); + return Promise.resolve(new ArrayBuffer(0)); },Also applies to: 249-251, 280-282, 314-316, 382-384, 414-416
924-926: 对象字面量中的arrayBuffer方法使用了错误的this上下文。在接受测试（acceptance tests）和
AcceptConfig测试中，使用对象字面量定义的文件 mock 使用箭头函数语法arrayBuffer: () => { return Promise.resolve(this); }。箭头函数不绑定自己的this上下文，因此this不会指向文件对象，而是指向外层作用域（可能是undefined或测试上下文）。应用此差异修复：
{ name: 'accepted.webp', - arrayBuffer: () => { - return Promise.resolve(this); - }, + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), },此模式应用于所有对象字面量中的 arrayBuffer 定义。
Also applies to: 930-932, 936-938, 950-952, 956-958, 970-972, 976-978, 982-984, 996-998, 1002-1004, 1008-1010, 1023-1025, 1030-1032, 1045-1047, 1052-1054, 1059-1061, 1074-1076, 1081-1083, 1096-1098, 1103-1105, 1118-1120, 1125-1127, 1141-1143, 1148-1150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/AjaxUploader.tsx(1 hunks)tests/uploader.spec.tsx(34 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-09T06:22:00.335Z
Learnt from: zombieJ
Repo: react-component/upload PR: 543
File: src/AjaxUploader.tsx:116-116
Timestamp: 2025-04-09T06:22:00.335Z
Learning: 在 react-component/upload 项目中，黏贴上传功能应该是可配置的，只有当启用此功能时才应添加 paste 事件监听器，而不是无条件添加。同时，事件监听器绑定应使用预先绑定的方法而非直接使用 .bind(this) 以避免内存泄漏。
Applied to files:
tests/uploader.spec.tsx
🧬 Code graph analysis (1)
tests/uploader.spec.tsx (1)
src/attr-accept.ts (1)
file(4-53)
There was a problem hiding this comment.
Code Review
This pull request addresses an error during the upload of cloud-based files by introducing a file caching mechanism. The core logic change in src/AjaxUploader.tsx involves reading the file into a buffer and creating a new File object to ensure consistency, which is a solid approach. The accompanying test updates in tests/uploader.spec.tsx are extensive but contain several issues in the mocking of the arrayBuffer method and the polyfill for Blob.prototype.arrayBuffer. My review provides suggestions to correct these test implementations, ensuring the new functionality is reliably tested.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/uploader.spec.tsx (3)
211-221: ajax 场景的 file mockarrayBuffer统一为 Promise 是可以接受的
- 各处上传相关的 mock（
upload success/error、drag、paste 等）都为“伪 File 对象”补上了arrayBuffer()，并返回Promise.resolve(this.name)：
- 虽然真实 API 里应返回
ArrayBuffer，但在生产代码中只是把await file.arrayBuffer()的结果作为new File([buffer], ...)的内容使用，而string本身就是合法的BlobPart，所以功能上是 OK 的。arrayBuffer定义为常规方法（而非箭头函数）并通过this.name取值，避免了过去提到的this绑定问题。- 将回调超时从
100ms调整到150ms只是为了给异步缓存步骤更多时间，对测试行为和语义没有负面影响。如果后续还需要新建类似 mock，可以考虑抽一个小工具函数（例如
createMockFile(name: string)）统一生成带arrayBuffer的对象，减少重复。Also applies to: 247-255, 279-287, 313-321, 371-374, 380-388, 412-420
922-1163: Accept / AcceptConfig 测试用例与过滤语义大体匹配，但建议确认自定义 filter 的预期
- 这里为所有用于
accept/AcceptConfig的对象都补充了arrayBuffer，确保这些文件在走统一缓存逻辑时不会抛错，同时name和type字段保持原有行为，测试重点仍然是格式过滤本身。- 测试覆盖了：
- 扩展名（
.png、.jpg,.jpeg等）、MIME（image/*、image/jpeg）、*/*/*等多种模式；- 非法模式（
'jpg'）下应仅输出 warning 而不阻止文件；directory: false场景下跳过 JS 层级 accept 校验。AcceptConfig部分中：
filter: 'native'时期待“原生过滤”绕过格式校验，这与注释一致。filter: (file) => file.name.includes('custom')并配合format: '.png'，当前断言只期望custom.jpg通过，看起来自定义 filter 的结果优先于 format。最后这一点（自定义 filter 是否应该“盖掉”
format）是行为设计问题，建议再确认一下与业务预期是否一致；如果设计上希望format依然生效，那么这里的期望值可能需要相应调整。Also applies to: 1195-1278
1335-1345: onBatchStart / beforeUpload 行为断言合理，但act的使用方式可以更规范
onBatchStart相关断言（batchEventFiles和包含parsedFile字段的版本）很好地验证了：
- 批量开始事件中每一项都携带原始
file信息（name、type）；- 当某个文件在
beforeUpload中被拒绝时，其parsedFile为null，其它通过的文件则有对应克隆后的parsedFile。dynamic change action in beforeUpload should work中：
- 通过在
beforeUpload里更新action并等待后续请求发出，来验证请求 URL 使用的是最新的action，测试目标清晰。- 这里直接在组件内部调用
await act(() => { setAction('bamboo'); });比较少见，而且fireEvent.change自身已经被 React Testing Library 用act包装，存在嵌套act的可能。建议的更“标准”的做法是把
act移到测试外层，例如：async function beforeUpload() { setAction('bamboo'); await sleep(100); return true; } // 测试里 await act(async () => { fireEvent.change(input, { target: { files: [...] } }); });这样可以避免在组件实现中直接依赖测试工具函数，同时减少潜在的 “nested act” 告警风险。请确认当前在 React 18 下运行测试不会产生 act 相关 warning。
Also applies to: 1400-1412, 1443-1453, 1464-1471
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/uploader.spec.tsx(34 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: zombieJ
Repo: react-component/upload PR: 543
File: src/AjaxUploader.tsx:116-116
Timestamp: 2025-04-09T06:22:00.335Z
Learning: 在 react-component/upload 项目中，黏贴上传功能应该是可配置的，只有当启用此功能时才应添加 paste 事件监听器，而不是无条件添加。同时，事件监听器绑定应使用预先绑定的方法而非直接使用 .bind(this) 以避免内存泄漏。
📚 Learning: 2025-04-09T06:22:00.335Z
Learnt from: zombieJ
Repo: react-component/upload PR: 543
File: src/AjaxUploader.tsx:116-116
Timestamp: 2025-04-09T06:22:00.335Z
Learning: 在 react-component/upload 项目中，黏贴上传功能应该是可配置的，只有当启用此功能时才应添加 paste 事件监听器，而不是无条件添加。同时，事件监听器绑定应使用预先绑定的方法而非直接使用 .bind(this) 以避免内存泄漏。
Applied to files:
tests/uploader.spec.tsx
🧬 Code graph analysis (1)
tests/uploader.spec.tsx (1)
src/attr-accept.ts (1)
file(4-53)
🔇 Additional comments (2)
tests/uploader.spec.tsx (2)
2-2: Blob.arrayBuffer polyfill 与 Item.arrayBuffer 的实现基本合理
Item构造函数中添加arrayBuffer = () => Promise.resolve(this.name)，现在返回的是string，属于合法的BlobPart，比之前返回整个对象本身要安全、贴近真实行为。beforeAll中只在环境缺少Blob.prototype.arrayBuffer时进行 polyfill，且实现使用FileReader.readAsArrayBuffer(this)，能够返回真正的ArrayBuffer，也修复了过去箭头函数导致的this绑定问题。唯一需要确认的是：当前 Jest / jsdom 运行环境中一定存在
FileReader，否则 polyfill 会在测试启动时抛错。如果有跑到非浏览器环境（如纯 Node，无 jsdom），可以考虑在 polyfill 中再加一层if (typeof FileReader === 'undefined')的保护或降级实现。Also applies to: 13-14, 89-99
598-639: 目录上传场景中为虚拟文件/目录补充arrayBuffer是合理的
- 目录上传相关测试里，顶层的
files目录对象和通过Item封装的“子文件”现在都能提供arrayBuffer()，与新的cacheFiles行为对齐，避免在遍历DataTransferItem/webkitGetAsEntry时出现缺失方法。- 对于通过
makeFileSystemEntry/makeFileSystemEntryAsync生成的层级结构，真正作为文件传入 uploader 的是new Item(item.name)，Item已经实现了arrayBuffer，因此测试覆盖路径是完整的。accept/ 类型过滤相关断言（例如只匹配.png，或在未匹配时不触发onStart）依旧根据name/type字段判断，没有被arrayBuffer改动影响。这些改动很好地避免了 cloud-only / 非真实
File对象在目录上传路径上被误判为错误状态。Also applies to: 650-667, 697-725, 740-778, 797-807, 823-829, 847-860
|
@yoyo837 @li-jia-nan Whenever you get a moment, can you take a look at this? |
1 participant
This PR fixes an issue that uploader throws an error when a file from cloud storage is uploaded
Close: ant-design/ant-design#55694
Summary by CodeRabbit
发版说明
重构
测试
✏️ Tip: You can customize this high-level summary in your review settings.