refactor: remove unnecessary eslint-disable and @ts-ignore#9439
refactor: remove unnecessary eslint-disable and @ts-ignore#9439chenshuai2144 merged 2 commits intoant-design:masterfrom
@ts-ignore#9439Conversation
…components - Cleaned up code by removing redundant eslint-disable comments related to array index keys and other rules in various components. - Improved code readability and maintainability by ensuring adherence to linting rules without compromising functionality. - Ensured that all components remain functional and visually consistent after the removal of these comments.
dosubot
bot
added
the
size:M
📝 WalkthroughWalkthrough本次提交主要移除大量 ESLint/TypeScript 抑制注释（如 exhaustive-deps、react/no-array-index-key、@ts-ignore 等），同时包含少量行为相关改动：保留了部分 props（emptyText/ref）、移除 Captcha 的 console.error、nanoid 增加了 window?.crypto?.randomUUID 检查、ListToolBar 明确添加了 keys、以及 Table 中删除 _timestamp 的写法微调。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 @zzjiaxiang, 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 is a significant refactoring effort aimed at enhancing the overall code quality and maintainability of the project. It systematically removes various unnecessary ESLint disable comments and TypeScript Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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
|
![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 primarily focuses on code quality improvements by removing numerous ESLint disable comments across various files. Specifically, it removes react-hooks/exhaustive-deps comments from many useEffect and useMemo hooks, react/no-array-index-key comments from several list rendering components, @ts-ignore comments to improve type safety, and no-param-reassign comments to prevent direct mutation of function parameters. Additionally, no-console and other general ESLint disable comments are removed. The review comments highlight that while removing exhaustive-deps is good, the underlying issues of stale closures for functions like fetchList in useFetchData.tsx need to be addressed by wrapping them in useRefFunction. It also points out missing dependencies in useEffect hooks, such as props.countDown in Captcha/index.tsx and props.onPageChange, location in ProLayout.tsx. Furthermore, the reviewer suggests refactoring code that uses delete on props objects (e.g., emptyText, ref) to instead create new objects without the unwanted properties, promoting immutability.
| } | ||
| fetchList(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [...(effects || []), manual]); |
There was a problem hiding this comment.
Removing the eslint-disable-next-line react-hooks/exhaustive-deps comment is good, but the underlying issue should be addressed. The fetchList function is redefined on every render, and it's not included in the useEffect dependency array. This can lead to stale closures.
To fix this, you can wrap fetchList in useRefFunction to ensure it's a stable reference. This utility is already available in the project.
const fetchList = useRefFunction(async () => {
// ... function body
});| } | ||
| return () => clearInterval(interval); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [timing]); |
There was a problem hiding this comment.
The useEffect hook depends on props.countDown, but it's not included in the dependency array. This can lead to using a stale value of countDown if the prop changes. Please add props.countDown to the dependency array.
| }, [timing]); | |
| }, [timing, props.countDown]); |
| useEffect(() => { | ||
| props.onPageChange?.(props.location); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [location.pathname, location.pathname?.search]); |
There was a problem hiding this comment.
The useEffect hook depends on props.onPageChange and props.location, but they are not included in the dependency array. This can lead to stale closures. Instead of depending on location.pathname, you can depend on location directly.
| }, [location.pathname, location.pathname?.search]); | |
| }, [location, props.onPageChange]); |
| } | ||
|
|
||
| // eslint-disable-next-line no-param-reassign | ||
| delete props.emptyText; |
There was a problem hiding this comment.
Mutating function arguments like the props object is generally a bad practice as it can lead to unexpected side effects in other parts of the application that might be using the same object. Instead of using delete, it's safer to create a new object without the emptyText property, for example by using object destructuring.
| valueTypeMap && valueTypeMap[valueType as string]; | ||
| if (customValueTypeConfig) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| delete props.ref; |
There was a problem hiding this comment.
Mutating function arguments like the props object is generally a bad practice as it can lead to unexpected side effects. Instead of using delete, it's safer to create a new object without the ref property, for example by using object destructuring.
| } | ||
|
|
||
| // eslint-disable-next-line no-param-reassign | ||
| delete props.emptyText; |
There was a problem hiding this comment.
Mutating function arguments like the props object is generally a bad practice as it can lead to unexpected side effects. Instead of using delete, it's safer to create a new object without the emptyText property, for example by using object destructuring.
|
|
||
| if (customValueTypeConfig) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| delete props.ref; |
There was a problem hiding this comment.
Mutating function arguments like the props object is generally a bad practice as it can lead to unexpected side effects. Instead of using delete, it's safer to create a new object without the ref property, for example by using object destructuring.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/form/layouts/DrawerForm/index.tsx (1)
185-193:⚠️ Potential issue | 🟠 Major移除
eslint-disable后，此useEffect缺少多个依赖项，ESLint 将会报警告。回调体中引用了
onOpenChange、setDrawerWidth、resizeInfo?.minWidth，但依赖数组仅为[propsOpen, open, resizableDrawer]。之前的eslint-disable就是用来抑制这些缺失依赖的警告。与 ModalForm 存在同样的问题——单纯删除抑制注释而不补全依赖，会导致 ESLint 检查失败。建议：
- 补全依赖数组（注意
onOpenChange可通过useRefFunction包装保持引用稳定，setDrawerWidth作为useState的 setter 引用本身是稳定的，可安全添加）；- 或者确认项目 CI 配置对该规则的严格程度。
♻️ 建议补全依赖数组
useEffect(() => { if (open && propsOpen) { onOpenChange?.(true); } if (resizableDrawer) { setDrawerWidth(resizeInfo?.minWidth); } - }, [propsOpen, open, resizableDrawer]); + }, [propsOpen, open, resizableDrawer, onOpenChange, setDrawerWidth, resizeInfo?.minWidth]);Based on learnings: "Pass all ESLint and TypeScript checks".
src/utils/nanoid/index.ts (1)
29-32:⚠️ Potential issue | 🟡 Minor使用严格相等判断
typeof结果
typeof crypto.randomUUID == 'function'建议改为严格相等，避免潜在的类型转换风险。✅ 建议修改
- typeof crypto.randomUUID == 'function' + typeof crypto.randomUUID === 'function'src/utils/useEditableArray/index.tsx (1)
762-765:⚠️ Potential issue | 🟡 Minor移除顶层
eslint-disable后，空依赖数组可能触发react-hooks/exhaustive-deps警告。
buildDataSourceKeyIndexMap虽然通过useRefFunction返回了稳定引用，但 ESLint 的 exhaustive-deps 规则无法识别这一点，会将其标记为缺失依赖。移除文件顶部的eslint-disable后，此处会产生 lint 告警。建议将
buildDataSourceKeyIndexMap加入依赖数组（因为它本身是稳定引用，不会导致额外重渲染），或者添加行级别的// eslint-disable-next-line react-hooks/exhaustive-deps注释。建议修改
const initDataSourceKeyIndexMap = useMemo( () => buildDataSourceKeyIndexMap(), - [], + // buildDataSourceKeyIndexMap is a stable ref from useRefFunction + // eslint-disable-next-line react-hooks/exhaustive-deps + [], );Based on learnings: "Pass all ESLint and TypeScript checks".
src/utils/useEditableMap/index.tsx (1)
264-300:⚠️ Potential issue | 🟡 Minor
actionRender的useCallback依赖数组不完整，移除顶层eslint-disable后会触发 lint 告警。
useCallback内部引用了saveText、cancelText、deleteText、intl、setEditableRowKeys、props.actionRender等变量，但依赖数组中未包含。之前由文件级别的eslint-disable react-hooks/exhaustive-deps抑制了此告警。建议补全依赖数组，或在此处添加行级
eslint-disable-next-line并附上原因注释。其中saveText/cancelText/deleteText依赖于intl（locale 变化时会更新），遗漏可能导致国际化切换后操作按钮文案未更新。建议补全依赖
const actionRender = useCallback( (key: RecordKey, config?: ActionTypeText<RecordType>) => { // ... }, - [editableKeys, props.dataSource, cancelEditable, onCancel, onSave], + [editableKeys, props.dataSource, cancelEditable, onCancel, onSave, saveText, cancelText, deleteText, setEditableRowKeys, intl, props.actionRender], );Based on learnings: "Pass all ESLint and TypeScript checks".
src/utils/isDeepEqualReact/index.ts (1)
14-14:⚠️ Potential issue | 🟠 Major使用严格相等比较
!==替代松散相等比较!=。既然已移除
eqeqeq的 ESLint 禁用指令，代码应遵循严格相等比较规范。虽然在此上下文中length是数字类型，但使用!==更符合最佳实践。运行以下脚本验证 ESLint 检查是否通过：
#!/bin/bash # 描述：验证移除 lint 指令后是否仍通过 ESLint 检查 # 仅检查此文件的 ESLint 状态 npx eslint src/utils/isDeepEqualReact/index.ts --format=json🔧 建议的修复
- if (length != b.length) return false; + if (length !== b.length) return false;- if (length != b.length) return false; + if (length !== b.length) return false;Also applies to: 38-38
🧹 Nitpick comments (3)
src/table/Table.tsx (1)
437-437: 移除as any的做法正确，但多余的括号可以一并清理。
actionParams的类型已经是Record<string, any>，移除as any完全没问题。不过(actionParams)的括号是之前(actionParams as any)的残留，可以顺手去掉。🔧 建议的修改
- delete (actionParams)._timestamp; + delete actionParams._timestamp;src/utils/useEditableArray/index.tsx (1)
1088-1091: 此处仍保留了@ts-ignore，与 PR 目标不一致。PR 的目标是移除不必要的
eslint-disable和@ts-ignore。此处的@ts-ignore是因为...rest: any[]展开传入props.onValuesChange时类型不匹配。可以通过显式声明参数类型来消除：建议修改
- const propsOnValuesChange = useDebounceFn(async (...rest: any[]) => { - //@ts-ignore - props.onValuesChange?.(...rest); + const propsOnValuesChange = useDebounceFn(async (record: RecordType, dataSource: RecordType[]) => { + props.onValuesChange?.(record, dataSource); }, 64);src/utils/merge/index.ts (1)
6-33: 建议改进类型定义和代码现代化。虽然此次改动仅移除了 ESLint 抑制指令，但代码本身存在一些可以改进的地方：
类型安全问题（第 6 行）：使用了
any类型作为泛型默认值和参数类型。根据编码规范，应避免使用any，尽可能精确定义类型。代码现代化（第 12-13 行）：使用
for...in循环配合hasOwnProperty是较旧的模式，可以使用Object.keys()或Object.entries()等现代方法。
hasOwnProperty安全性（第 13 行）：直接调用rest[i].hasOwnProperty(key)在某些边缘情况下可能不安全（对象可能重写了hasOwnProperty），建议使用Object.prototype.hasOwnProperty.call(rest[i], key)。💡 可选的类型安全和现代化改进方案
/** * 用于合并 n 个对象 - * `@param` {any[]} ...rest - * `@returns` T + * `@param` {...Record<string, any>[]} rest - 要合并的对象数组 + * `@returns` {T} 合并后的对象 */ -const merge = <T = any>(...rest: any[]): T => { +const merge = <T extends Record<string, any> = Record<string, any>>( + ...rest: Array<Partial<T> | Record<string, any>> +): T => { const obj = {} as Record<string, any> as any; - const il = rest.length; - let key; - let i = 0; - for (; i < il; i += 1) { - for (key in rest[i]) { - if (rest[i].hasOwnProperty(key)) { + for (const item of rest) { + for (const key of Object.keys(item)) { + if (Object.prototype.hasOwnProperty.call(item, key)) { if ( typeof obj[key] === 'object' && - typeof rest[i][key] === 'object' && + typeof item[key] === 'object' && obj[key] !== undefined && obj[key] !== null && !Array.isArray(obj[key]) && - !Array.isArray(rest[i][key]) + !Array.isArray(item[key]) ) { obj[key] = { ...obj[key], - ...rest[i][key], + ...item[key], }; } else { - obj[key] = rest[i][key]; + obj[key] = item[key]; } } } } return obj as T; };这些改进将：
- 提供更好的类型约束和推断
- 使用现代化的迭代方式
- 增强
hasOwnProperty调用的安全性
- Updated the nanoid function to streamline the UUID generation process by consolidating conditional checks. - Improved code readability by replacing verbose checks with optional chaining and a single return statement. - Maintained functionality by ensuring compatibility with both randomUUID and genNanoid methods.
2 participants
Summary by CodeRabbit
发布说明
代码质量改进
行为调整 / 修复
其他优化