-
Notifications
You must be signed in to change notification settings - Fork 491
fix(Indent): Used deep check for props equality in Indent component #961
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?
fix(Indent): Used deep check for props equality in Indent component #961
Conversation
Previously shallow equality was used which ended up rerendering Indent even if props were the same
Walkthrough此次更改在 React 组件 Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/Indent.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs+fabric@4.0.1_jest@30.0.4_@types+node@24.0.10__postcss@8.5.6/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/utils/diffUtil.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs+fabric@4.0.1_jest@30.0.4_@types+node@24.0.10__postcss@8.5.6/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Indent.tsx
(2 hunks)src/utils/diffUtil.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Indent.tsx (1)
src/utils/diffUtil.ts (1)
areArraysEqual
(47-49)
🔇 Additional comments (3)
src/Indent.tsx (3)
3-3
: 导入语句正确正确导入了新的工具函数,支持深度数组比较功能。
34-41
: 自定义比较函数实现得当
arePropsEqual
函数正确实现了所有属性的比较:
- 对基本类型属性使用浅比较(
prefixCls
,level
)- 对数组属性使用深度比较(
isStart
,isEnd
)这个实现完全符合 PR 目标,能够有效防止由于数组引用变化但内容相同导致的不必要重渲染。
43-43
: React.memo 使用正确正确地将自定义比较函数传递给
React.memo
,这将解决原始问题中由于浅比较导致的性能问题。
export function areArraysEqual(a: any[], b: any[]) { | ||
return a.length === b.length && a.every((val, i) => val === b[i]); | ||
} |
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.
🛠️ Refactor suggestion
建议改进类型安全性和边界情况处理
当前实现存在以下问题:
- 使用
any[]
类型降低了类型安全性 - 缺乏对
null
或undefined
的处理,可能导致运行时错误
建议应用以下改进:
-export function areArraysEqual(a: any[], b: any[]) {
- return a.length === b.length && a.every((val, i) => val === b[i]);
+export function areArraysEqual<T>(a: T[], b: T[]): boolean {
+ if (a === b) return true;
+ if (!a || !b) return false;
+ return a.length === b.length && a.every((val, i) => val === b[i]);
这样可以:
- 提供更好的类型安全性
- 处理相同引用的快速路径
- 安全处理 null/undefined 情况
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export function areArraysEqual(a: any[], b: any[]) { | |
return a.length === b.length && a.every((val, i) => val === b[i]); | |
} | |
export function areArraysEqual<T>(a: T[], b: T[]): boolean { | |
if (a === b) return true; | |
if (!a || !b) return false; | |
return a.length === b.length && a.every((val, i) => val === b[i]); | |
} |
🤖 Prompt for AI Agents
In src/utils/diffUtil.ts around lines 47 to 49, the function areArraysEqual uses
any[] which reduces type safety and does not handle null or undefined inputs,
risking runtime errors. Update the function to use generic types for better type
safety, add a quick check for reference equality to optimize performance, and
include explicit checks to safely handle cases where either input is null or
undefined before proceeding with element-wise comparison.
Summary by CodeRabbit