-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
chore: [WIP] update @rc-component/upload #600
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
""" 概述演练这个拉取请求包含了对 变更
诗歌
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 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)
Summary by CodeRabbit
发布说明
依赖更新
rc-upload
更改为@rc-component/upload
1.0.0
@rc-component/util
的依赖代码重构
Upload
和AjaxUploader
组件从类组件转换为函数组件类型改进
其他改进
pnpm-lock.yaml
的忽略规则