-
-
Notifications
You must be signed in to change notification settings - Fork 320
chore: update @rc-component/upload #600
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
概述演练这个拉取请求包含了对 变更
诗歌
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🔇 Additional comments (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
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
🧹 Nitpick comments (4)
src/AjaxUploader.tsx (2)
91-93
: 避免在类型定义中使用void
以防混淆。静态分析提示在联合类型中使用
void
可能导致理解混淆,建议改用undefined
更易维护。- ) => BeforeUploadFileType | Promise<void | BeforeUploadFileType> | void; + ) => BeforeUploadFileType | Promise<undefined | BeforeUploadFileType> | undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 93-93: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
194-209
:uploadFiles
函数逻辑清晰,但可添加并发管理。如果文件数量过大或网络条件复杂,建议引入并发或队列控制,避免一次性发起过多请求导致卡顿或拥塞。
src/Upload.tsx (1)
26-42
: 将属性透传给AjaxUpload
确保了可扩展性。通过
{...rest}
传递其它props
,在保持组件灵活性的同时,也需注意对无效属性的过滤或文档说明。tests/uploader.spec.tsx (1)
Line range hint
575-598
: 错误处理测试完善添加了文件读取错误的测试场景,但建议添加更多断言来验证错误处理的具体行为。
+ expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('read file error') + );🧰 Tools
🪛 GitHub Actions: ✅ test
[error] 570-570: Test assertion failed: Expected 2 but received 1 in file reading test
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.gitignore
(1 hunks)docs/examples/beforeUpload.tsx
(1 hunks)docs/examples/customRequest.tsx
(1 hunks)package.json
(3 hunks)src/AjaxUploader.tsx
(6 hunks)src/Upload.tsx
(1 hunks)src/attr-accept.ts
(1 hunks)src/interface.tsx
(1 hunks)src/request.ts
(2 hunks)src/traverseFileTree.ts
(1 hunks)src/uid.ts
(1 hunks)tests/uploader.spec.tsx
(6 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/attr-accept.ts
- src/uid.ts
- docs/examples/customRequest.tsx
- docs/examples/beforeUpload.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
tests/uploader.spec.tsx
[error] 59-59: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/AjaxUploader.tsx
[error] 93-93: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🪛 GitHub Actions: ✅ test
tests/uploader.spec.tsx
[error] 96-97: React state updates in tests are not wrapped in act(...). Updates should be wrapped in act(() => { /* updates */ }) to ensure proper testing behavior.
[error] 224-224: TypeError: Cannot read properties of undefined (reading 'respond') in upload success test
[error] 570-570: Test assertion failed: Expected 2 but received 1 in file reading test
[error] 1067-1067: TypeError: Cannot read properties of undefined (reading 'url') in dynamic action change test
src/AjaxUploader.tsx
[warning] 57-296: Low code coverage (73.04%). Multiple sections need additional test coverage, including lines 57-63, 71-72, 99, 157, 176-186, 223-233, 237-238, 245, 296
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (15)
src/AjaxUploader.tsx (4)
2-3
: 导入依赖用法恰当,暂无需修改。这两条
import
语句符合常规用法,未发现不当之处。
54-78
: 封装中止请求逻辑时,建议补充单测覆盖。当前
abort
函数可处理单个文件或全部文件的上传中止,但从流水线日志可知对应方法的测试覆盖率不足,建议编写或完善相关测试用例以提升稳定性。
211-219
:onChange
触发重新生成uid
逻辑合理,但需加强测试覆盖。从流水线输出可见此处的测试覆盖率偏低,尤其在多文件场景下,建议新增测试来确保不同上传状态下逻辑正确。
283-309
: JSX 结构直观,建议保持组件外层标签的可访问性。目前返回的
Tag
元素可当作 button 角色,需确保在实际使用时无障碍特性(例如键盘导航)得到充分测试。src/Upload.tsx (1)
7-24
: 使用函数式组件并解构默认值简洁易懂。该写法减少了类组件的冗余,默认参数处理得当,可读性更高。
src/interface.tsx (1)
37-39
: 事件类型放宽至HTMLElement
,提升了通用性。这一更改可在更多场景使用,避免局限于
div
标签。建议同步关注外部对这些事件的使用,确保兼容性完好。src/traverseFileTree.ts (2)
17-20
: 代码格式优化将 forEach 循环的箭头函数展开为块级语句,提高了代码的可读性。
24-24
: 类型安全性增强为
entries
变量添加了明确的类型注解InternalDataTransferItem[]
,并优化了 Promise 解析的格式,提高了代码的类型安全性。Also applies to: 27-31
src/request.ts (2)
20-22
: 简化错误处理逻辑在 JSON 解析的 catch 块中移除了未使用的错误参数,使代码更简洁。
25-25
: 导出结构优化将
upload
函数的导出方式改为先声明后导出的形式,提高了代码的组织性。Also applies to: 108-109
tests/uploader.spec.tsx (1)
380-391
: 测试用例期望值更新测试用例中的文件对象验证更加详细和明确。
.gitignore (1)
31-31
: 包管理器锁定文件忽略添加
pnpm-lock.yaml
到 .gitignore 文件中,符合项目使用 pnpm 作为包管理器的实践。package.json (3)
2-3
: 包名和版本号的重大变更需要注意将包名从
rc-upload
更改为@rc-component/upload
并重置版本号到1.0.0
是一个破坏性变更。请确保:
- 在发布前更新 CHANGELOG
- 在 README 中说明迁移指南
- 考虑使用
deprecate
标记旧包名
41-42
: 依赖项更新看起来合理将
rc-util
替换为@rc-component/util
符合组件库的整体规划。
49-52
: 依赖版本存在潜在风险
- React 19 尚未正式发布,使用预发布版本可能存在稳定性风险
- Node.js 22 尚未发布,但
@types/node
已指定为^22.12.0
- 建议:
- 暂时保持使用 React 18
- 将 Node types 版本改为当前 LTS 版本
Also applies to: 66-67
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
🧹 Nitpick comments (2)
src/AjaxUploader.tsx (2)
54-78
: abort 函数实现逻辑清晰,但可优化状态更新
在条件分支中多次调用 setReqs 进行状态更新,建议合并更新以减少不必要的状态变更。
280-313
: JSX 渲染结构总体清晰
使用 Tag 组件作为容器并合理分配各项属性,通过 pickAttrs 分发 aria 与 data 属性。但需注意:
在 元素上使用的className={classNames.input}
可能与导入的 classnames 库名称产生混淆,请确认传入的 classNames prop 结构符合预期。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/AjaxUploader.tsx
(6 hunks)src/Upload.tsx
(1 hunks)src/uid.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/uid.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/AjaxUploader.tsx
[error] 93-93: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (18)
src/Upload.tsx (4)
1-3
: 导入部分正确无误
React、AjaxUpload 以及对应类型的导入均符合预期,没有冗余或遗漏。
7-24
: 组件属性解构与默认值设置合理
通过解构 props 并设置默认参数(例如 component、prefixCls 等),使组件配置直观且易于使用。
25-43
: JSX 渲染结构清晰
直接使用 AjaxUpload 组件传递所有必要的属性,结构简单直接,便于后续维护和扩展。
46-47
: displayName 设置恰当
仅在非生产环境下为组件设置 displayName,便于开发调试,同时不会影响生产环境。src/AjaxUploader.tsx (14)
2-4
: 依赖导入管理得当
引入的 classnames、pickAttrs、React 及其他依赖均符合组件需求,确保了代码的可读性和稳定性。
25-47
: 组件属性解构与重命名使用得当
将 props 中的 component 重命名为 Tag 并提取其他需要的属性,这种写法符合现代 React 函数组件的最佳实践。
48-53
: 状态与引用初始化正确
通过 useState 初始化 uid 与 reqs,利用 useRef 管理 isMounted 和 input 引用,符合 React Hooks 的设计理念。
80-86
: useEffect 钩子使用合理
在组件挂载时设置 isMountedRef,并在卸载时调用 abort 取消未完成请求,有效管理了异步行为的清理。
Line range hint
91-153
: processFile 异步处理逻辑清晰
通过 try-catch 处理 beforeUpload,并根据返回结果判断是否继续上传,逻辑严谨。请确保上层调用方能妥善处理返回的 null 值情况。🧰 Tools
🪛 Biome (1.9.4)
[error] 93-93: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
Line range hint
155-192
: post 方法实现合理
构建请求选项时包含 onProgress、onSuccess、onError 等回调,上传请求后能及时从 reqs 中移除对应记录,整体逻辑清晰。
194-213
: uploadFiles 方法设计得当
先对文件进行预处理(包括更新 uid 与调用 processFile),再使用 Promise.all 执行批量上传,充分利用异步执行特性。
215-222
: onChange 事件处理得当
在文件变化时正确过滤出符合条件的文件,调用 uploadFiles 发起上传,同时更新 uid 以重置 input 组件。
224-238
: onClick 事件逻辑完善
通过判断 target 标签、处理焦点问题并触发 input 点击,提升了组件的交互体验和可访问性。
240-243
: onKeyDown 实现基本键盘交互
捕获 Enter 键并调用 onClick 方法,确保了键盘操作与鼠标点击行为的一致性。
246-264
: onFileDrop 文件拖拽处理妥当
针对目录与普通文件分别处理文件拖拽事件,调用 traverseFileTree 或直接过滤 files,逻辑完备。
266-267
: 目录属性赋值正确
根据 directory 属性设置 input 的 directory 和 webkitdirectory 属性,保证文件夹上传的兼容性。
269-278
: 事件绑定灵活配置
根据 disabled 状态条件性绑定 onClick、onKeyDown、onDrop 等事件,确保组件在不同状态下的交互行为正确。
316-317
: displayName 设置符合开发需求
在非生产环境下为 AjaxUploader 设置 displayName,便于调试与组件识别,处理得当。
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 (3)
src/AjaxUploader.tsx (3)
49-51
: 建议优化 reqs 的类型定义当前 reqs 使用了
Record<PropertyKey, any>
,建议定义更具体的类型以提高类型安全性。例如:-const [reqs, setReqs] = React.useState<Record<PropertyKey, any>>({}); +const [reqs, setReqs] = React.useState<Record<PropertyKey, { abort?: () => void }>>({});
225-239
: 建议优化 onClick 处理函数当前的按钮焦点处理逻辑可以简化,同时提高可读性:
const onClick = ( event: React.MouseEvent<HTMLElement, MouseEvent> | React.KeyboardEvent<HTMLElement>, ) => { if (!inputRef.current) { return; } - const target = event.target as HTMLElement; - if (target?.tagName === 'BUTTON') { - const parent = inputRef.current.parentNode as HTMLInputElement; - parent.focus(); - target.blur(); - } + if (event.target instanceof HTMLButtonElement) { + inputRef.current.parentNode?.focus(); + event.target.blur(); + } inputRef.current.click(); props.onClick?.(event); };
288-308
: 建议提取 input 属性以提高可读性建议将 input 的属性对象提取到渲染逻辑之外,这样可以使代码更清晰:
+const getInputProps = () => ({ + ...pickAttrs(otherProps, { aria: true, data: true }), + id, + name, + disabled, + type: "file", + onClick: (e: React.MouseEvent) => e.stopPropagation(), + style: { display: 'none', ...styles.input }, + className: classNames.input, + accept, + ...dirProps, + multiple, + ...(capture != null ? { capture } : {}), +}); return ( <Tag {...events} style={style} role={disabled || hasControlInside ? undefined : 'button'} tabIndex={disabled || hasControlInside ? undefined : 0} className={classnames(prefixCls, className, { [`${prefixCls}-disabled`]: disabled, })} > - <input - {...pickAttrs(otherProps, { aria: true, data: true })} - id={id} - name={name} - disabled={disabled} - type="file" - ref={inputRef} - onClick={e => e.stopPropagation()} - key={uid} - style={{ display: 'none', ...styles.input }} - className={classNames.input} - accept={accept} - {...dirProps} - multiple={multiple} - onChange={onChange} - {...(capture != null ? { capture } : {})} - /> + <input {...getInputProps()} ref={inputRef} key={uid} onChange={onChange} /> {children} </Tag> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/AjaxUploader.tsx
(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/AjaxUploader.tsx
[error] 94-94: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (2)
src/AjaxUploader.tsx (2)
26-47
: 代码结构清晰,类型定义完善!组件声明和属性解构遵循了 React 最佳实践,使用了 Readonly 和 PropsWithChildren 来确保类型安全。
92-94
: 类型定义建议优化变量 transformedFile 定义时采用了
BeforeUploadFileType | void
,建议使用undefined
替换 void,以提高类型清晰度。🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
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 (2)
src/AjaxUploader.tsx (2)
213-220
: 建议增强错误处理在文件上传过程中建议添加 try-catch 块来处理可能的异常,并通过 props 提供的错误回调通知上层组件。
const onChange = (e: React.ChangeEvent<HTMLInputElement>) => { + try { const { files } = e.target; const acceptedFiles = [...files].filter( (file: RcFile) => !directory || attrAccept(file, accept), ); uploadFiles(acceptedFiles); setUid(getUid()); + } catch (error) { + props.onError?.(error as UploadRequestError, null, null); + } };
275-308
: 建议增强无障碍性虽然已经添加了基本的无障碍属性,但建议增加
aria-label
来提供更好的屏幕阅读器支持。<input {...pickAttrs(otherProps, { aria: true, data: true })} id={id} name={name} disabled={disabled} type="file" + aria-label={`${name || '文件'}上传输入框`} ref={inputRef} onClick={e => e.stopPropagation()} key={uid} style={{ display: 'none', ...styles.input }} className={classNames.input} accept={accept} {...dirProps} multiple={multiple} onChange={onChange} {...(capture != null ? { capture } : {})} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/AjaxUploader.tsx
(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/AjaxUploader.tsx
[error] 91-91: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (3)
src/AjaxUploader.tsx (3)
26-47
: 代码结构清晰,类型定义完善!组件声明和属性解构的实现非常规范,使用了 React.FC 类型和只读属性,提高了代码的类型安全性。
49-84
: 状态管理实现规范!使用 useState 和 useRef 进行状态管理,并在 useEffect 中正确处理组件的挂载状态和清理工作,符合 React Hooks 的最佳实践。
89-91
: 类型定义建议优化变量 transformedFile 定义时采用了
BeforeUploadFileType | void
,建议使用undefined
替换 void,以提高类型清晰度。🧰 Tools
🪛 Biome (1.9.4)
[error] 91-91: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
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 (3)
src/AjaxUploader.tsx (3)
55-70
: 建议优化 abort 函数的依赖项abort 函数使用了 useCallback 但没有声明任何依赖项。虽然这个函数不依赖任何外部变量,但为了代码的可维护性,建议显式声明空依赖数组。
-}, []); +}, []); // 显式声明空依赖数组
232-246
: 建议增加文件读取错误处理在处理文件拖放时,建议添加 try-catch 块来处理可能的文件读取错误,提高代码的健壮性。
const onFileDrop = async (e: React.DragEvent<HTMLDivElement>) => { e.preventDefault(); if (e.type === 'dragover') { return; } - if (directory) { - const files = await traverseFileTree( - Array.prototype.slice.call(e.dataTransfer.items), - (f: RcFile) => attrAccept(f, accept), - ); - uploadFiles(files); - } else { - const allFiles = [...e.dataTransfer.files].filter((file: RcFile) => attrAccept(file, accept)); - uploadFiles(multiple === false ? allFiles.slice(0, 1) : allFiles); + try { + if (directory) { + const files = await traverseFileTree( + Array.prototype.slice.call(e.dataTransfer.items), + (f: RcFile) => attrAccept(f, accept), + ); + uploadFiles(files); + } else { + const allFiles = [...e.dataTransfer.files].filter((file: RcFile) => attrAccept(file, accept)); + uploadFiles(multiple === false ? allFiles.slice(0, 1) : allFiles); + } + } catch (error) { + console.error('文件读取失败:', error); + // 可以在这里添加错误处理逻辑,比如显示错误提示等 } };
286-286
: 建议提取内联样式建议将内联样式提取到样式对象或样式文件中,以提高代码的可维护性。
- style={{ display: 'none', ...styles.input }} + style={styles.input}然后在组件外部定义:
const defaultStyles = { input: { display: 'none', }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/AjaxUploader.tsx
(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/AjaxUploader.tsx
[error] 85-85: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (2)
src/AjaxUploader.tsx (2)
26-47
: 代码结构清晰,类型定义完善!组件定义和属性解构遵循了 React 最佳实践,使用了 Readonly 确保不可变性,类型定义完整。
83-85
: 优化类型定义变量 transformedFile 的类型定义使用了
BeforeUploadFileType | void
,建议使用undefined
替代 void,以提高类型的清晰度。- let transformedFile: BeforeUploadFileType | void = file; + let transformedFile: BeforeUploadFileType | undefined = file;🧰 Tools
🪛 Biome (1.9.4)
[error] 85-85: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
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 (3)
src/AjaxUploader.tsx (3)
1-1
: 考虑移除 ESLint 禁用注释禁用
react-hooks/exhaustive-deps
规则可能会隐藏依赖项问题,导致难以察觉的 bug。建议重新评估这些 hook 的依赖项,确保它们正确声明。
199-206
: 考虑清除 input 值当前
onChange
处理程序在上传文件后不会清除 input 的值,这可能导致用户无法连续上传相同的文件。建议考虑在上传完成后重置 input。const onChange = (e: React.ChangeEvent<HTMLInputElement>) => { const { files } = e.target; const acceptedFiles = [...files].filter( (file: RcFile) => !directory || attrAccept(file, accept), ); uploadFiles(acceptedFiles); setUid(getUid()); + // 重置 input 以支持上传相同文件 + e.target.value = ''; };
250-259
: 优化条件事件处理当
openFileDialogOnClick
为 false 时,onClick 和 onKeyDown 使用了空箭头函数。为了提高效率,可以考虑使用条件扩展运算符来完全省略这些处理程序。const events = disabled ? {} : { - onClick: openFileDialogOnClick ? onClick : () => {}, - onKeyDown: openFileDialogOnClick ? onKeyDown : () => {}, + ...(openFileDialogOnClick ? { onClick, onKeyDown } : {}), onMouseEnter, onMouseLeave, onDrop: onFileDrop, onDragOver: onFileDrop, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/AjaxUploader.tsx
(3 hunks)src/Upload.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/AjaxUploader.tsx
[error] 92-92: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (7)
src/Upload.tsx (2)
7-44
: 良好的函数组件重构!从类组件重构为函数组件的实现非常干净和清晰。使用解构赋值提供默认值比使用静态 defaultProps 更加现代化且类型安全。
46-48
: 良好的调试支持在非生产环境下设置 displayName 是一个很好的做法,这对于 React DevTools 中的组件调试非常有帮助。
src/AjaxUploader.tsx (5)
92-92
: 使用 undefined 替代 void在联合类型中使用 void 可能会引起混淆。根据静态分析工具的建议,使用 undefined 来提高类型的清晰度。
- let transformedFile: BeforeUploadFileType | void = file; + let transformedFile: BeforeUploadFileType | undefined = file;🧰 Tools
🪛 Biome (1.9.4)
[error] 92-92: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
63-78
: 检查 useCallback 的依赖项当前
abort
函数的依赖数组为空,这意味着即使reqsRef
发生变化,该函数也不会重新创建。虽然这可能是有意为之(因为使用了 ref),但建议确认这是否符合预期行为。
157-180
: 明确的请求选项结构请求选项对象的结构清晰,并正确地处理了文件上传的各种回调。这种实现方式有助于维护代码的可读性和可维护性。
80-86
: 适当的生命周期管理使用 useEffect 进行组件挂载状态的管理和清理操作是一个很好的做法。空依赖数组确保效果只在组件挂载和卸载时运行,这符合预期行为。
261-292
: 组件渲染结构良好渲染逻辑清晰地处理了各种边缘情况,包括辅助功能支持(role 和 tabIndex)以及条件类名。使用 pickAttrs 来过滤和传递适当的属性也是一个很好的做法。
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
🔭 Outside diff range comments (1)
src/AjaxUploader.tsx (1)
90-146
:⚠️ Potential issue将类方法重构为函数组件内部函数
processFile 函数的实现保持了原有逻辑,同时添加了 async/await 语法使代码更加清晰。
根据静态分析工具的提示,第 91 行中的联合类型使用了
void
,这在联合类型中可能会引起混淆:-let transformedFile: BeforeUploadFileType | void = file; +let transformedFile: BeforeUploadFileType | undefined = file;void 在类型系统中表示"无返回值",而 undefined 更准确地表示"可能没有值"。
🧰 Tools
🪛 Biome (1.9.4)
[error] 91-91: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🧹 Nitpick comments (3)
src/AjaxUploader.tsx (3)
1-4
: 导入语句变更和 ESLint 配置
- 添加了 ESLint 配置来禁用 react-hooks/exhaustive-deps 规则,但建议针对具体的 useEffect 或 useCallback 使用 eslint-disable-next-line 更为精准。
- 将
classNames
改为classnames
确保了导入名称与包名一致,提高代码可读性。- 更新了 pickAttrs 的导入路径,使用了新的 @rc-component/util 库路径。
建议考虑只在特定的 hooks 上禁用 exhaustive-deps 规则,而不是整个文件:
-/* eslint-disable react-hooks/exhaustive-deps */
然后在特定的 useEffect 或 useCallback 上使用行级禁用:
// 例如: // eslint-disable-next-line react-hooks/exhaustive-deps React.useEffect(() => { // ... }, []);
62-77
: 使用 useCallback 重构中止上传功能abort 方法的实现很好地使用了 useCallback 来优化性能,并正确处理了文件中止逻辑。
由于没有依赖项会更改,建议在 useCallback 中添加 reqsRef 作为依赖项,或者添加注释说明为什么依赖项是空数组:
-}, []); +// reqsRef is a stable reference, so it doesn't need to be a dependency +}, []);
79-85
: 使用 useEffect 替代生命周期方法使用 useEffect 替代 componentDidMount 和 componentWillUnmount 是符合现代 React 实践的好方法。这个 hook 正确地处理了组件的挂载状态跟踪和清理工作。
同样,应该在 useEffect 的依赖数组中添加 abort 函数,或添加注释说明为什么依赖项是空数组:
-}, []); +// abort is a stable reference created with useCallback +}, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/AjaxUploader.tsx
(3 hunks)src/Upload.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/Upload.tsx (3)
src/interface.tsx (1)
UploadProps
(7-47)docs/examples/beforeUpload.tsx (4)
onStart
(9-11)onError
(15-17)onSuccess
(12-14)beforeUpload
(18-27)docs/examples/customRequest.tsx (4)
onStart
(14-16)onError
(20-22)onSuccess
(17-19)customRequest
(26-65)
src/AjaxUploader.tsx (1)
src/interface.tsx (6)
UploadProps
(7-47)RcFile
(78-80)BeforeUploadFileType
(3-3)UploadRequestOption
(65-76)UploadProgressEvent
(49-51)UploadRequestError
(59-63)
🪛 Biome (1.9.4)
src/AjaxUploader.tsx
[error] 91-91: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (14)
src/Upload.tsx (3)
1-3
: 导入语句优化,使用类型导入将 UploadProps 使用 type 关键字导入是一个良好的做法,这可以确保类型信息在编译时被删除,不会影响运行时性能。
7-47
: 组件重构为函数式组件,使用现代 React 模式将类组件重构为函数式组件是一个很好的改进:
- 使用解构赋值和默认值代替了静态 defaultProps
- 代码更加简洁和声明式
- 遵循了 React 的最新实践
这种重构不仅使代码更加简洁,还有助于在未来的 React 版本中获得更好的性能优化。
49-51
: 添加了非生产环境的 displayName在非生产环境中设置 displayName 是一个很好的做法,这对 React DevTools 调试非常有帮助。
src/AjaxUploader.tsx (11)
12-12
: 导入 UploadRequestOption 类型导入 UploadRequestOption 类型,与函数组件重构相匹配,使类型更加明确。
25-54
: 组件重构为函数式组件,使用 React.FC 类型将类组件重构为函数式组件是一个很好的现代化改进:
- 使用 React.FC 类型增强了类型安全
- 使用 Readonly 提高了 props 的不可变性,防止意外修改
- 通过解构赋值使代码更加清晰
函数参数的解构非常全面,涵盖了所有必要的属性,并使用了 rest 操作符收集其余属性。
56-60
: 使用 React Hooks 管理状态和引用很好地使用了 React Hooks 替代类组件中的状态和实例变量:
- useState 替代了 this.state
- useRef 替代了实例变量,用于跟踪组件挂载状态和存储请求引用
这种方法更符合 React 的函数式编程范式。
148-179
: 重构 post 方法为函数组件内部函数post 函数很好地使用了组件挂载状态检查,并正确处理了上传请求的逻辑。
使用可选链操作符(?.)调用回调函数是一个很好的实践,这样可以避免在回调未定义时出现错误。
181-196
: 重构 uploadFiles 方法为函数组件内部函数uploadFiles 函数保持了原有逻辑,同时使代码更加简洁。Promise.all 的使用很好地处理了批量上传文件的异步操作。
198-205
: 重构 onChange 事件处理函数onChange 函数的实现清晰简洁,正确处理了文件筛选和上传,并通过 setUid 更新状态触发重新渲染。
207-221
: 重构 onClick 事件处理函数onClick 函数的实现考虑了按钮元素的特殊处理,确保了焦点管理和事件传播的正确性。
223-227
: 重构 onKeyDown 事件处理函数onKeyDown 函数正确处理了键盘事件,确保了可访问性,使用户可以通过键盘触发上传操作。
229-244
: 重构 onFileDrop 事件处理函数onFileDrop 函数正确实现了拖放文件上传功能,同时考虑了目录上传和文件筛选的逻辑。使用 async/await 使异步逻辑更加清晰。
246-288
: 渲染逻辑重构渲染逻辑被重构为直接的 JSX 返回语句,同时通过条件渲染和解构赋值使代码更加简洁和可读。值得注意的是:
- 添加了对目录上传的属性处理
- 使用条件对象构造事件处理程序
- 提供了适当的 ARIA 属性支持可访问性
- 文件输入元素的样式和行为设置合理
注释清晰地解释了代码的目的和解决的问题,引用了相关的 GitHub issues。
294-296
: 添加了非生产环境的 displayName在非生产环境中设置 displayName 是一个很好的做法,这对 React DevTools 调试非常有帮助。
Summary by CodeRabbit
发布说明
依赖更新
rc-upload
更改为@rc-component/upload
1.0.0
@rc-component/util
的依赖代码重构
Upload
和AjaxUploader
组件从类组件转换为函数组件类型改进
其他改进
pnpm-lock.yaml
的忽略规则