close
Skip to content

refactor: rewrite TreeNode CC => FC#905

Merged
afc163 merged 11 commits into
masterfrom
cc-2-fc
Jan 2, 2025
Merged

refactor: rewrite TreeNode CC => FC#905
afc163 merged 11 commits into
masterfrom
cc-2-fc

Conversation

@li-jia-nan
Copy link
Copy Markdown
Member

@li-jia-nan li-jia-nan commented Jan 1, 2025

Summary by CodeRabbit

  • 重构

    • 将树组件从类组件重构为功能性组件
    • 使用 React Hooks 优化状态管理和性能
    • 简化拖放操作的处理逻辑
    • 更新类型定义以提高类型安全性
  • 改进

    • 优化节点拖放事件的处理方式
    • 提升代码可读性和可维护性
    • 使用更现代的 React 最佳实践

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 1, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tree ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 2, 2025 8:25am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 1, 2025

概述

演练

此次拉取请求主要修改了树组件的拖放功能和类型定义。主要变更包括将NodeInstance类型替换为TreeNodeProps,重构TreeTreeNode组件,并更新相关的上下文类型和实用函数。这些修改旨在提高代码的类型安全性和可读性,同时保持组件的核心功能不变。

变更

文件 变更摘要
src/Tree.tsx - 移除NodeInstance类型
- 将dragNode替换为dragNodeProps
- 更新拖放事件处理方法的参数类型
src/TreeNode.tsx - 将InternalTreeNode类组件转换为函数组件
- 使用React Hooks重构状态管理
- 简化渲染逻辑
src/contextTypes.ts - 更新类型导入
- 修改NodeDragEventHandler签名
- 显式定义TreeContext类型
src/util.tsx - 替换NodeInstanceTreeNodeProps
- 更新calcDropPositionparseCheckedKeys函数

可能相关的拉取请求

  • chore: code optimization #906: 此PR也涉及对src/Tree.tsx中树组件的修改,特别关注拖放功能和类型注解,这与主PR中的更改直接相关。

诗歌

🐰 拖拽树,代码飞舞
类型更新,逻辑优雅
函数组件,Hooks轻盈
重构之路,兔子欢唱
代码如诗,跃然纸上 🌿


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.84%. Comparing base (fdb35f9) to head (461abdc).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #905      +/-   ##
==========================================
- Coverage   99.84%   99.84%   -0.01%     
==========================================
  Files          14       14              
  Lines        1326     1277      -49     
  Branches      397      392       -5     
==========================================
- Hits         1324     1275      -49     
  Misses          2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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/TreeNode.tsx (1)

43-43: 非稳定上下文使用
UnstableContext 仅用于试验性需求时可以接受,但要注意后续可能需要稳定 API 替换的风险。

src/Tree.tsx (1)

583-583: 替换 hasOwnProperty
此处若要使用 props.hasOwnProperty(name),请考虑用 Object.hasOwn() 替代,以避免 noPrototypeBuiltins 的风险。

🧰 Tools
🪛 Biome (1.9.4)

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ade840a and 0062e2b.

📒 Files selected for processing (4)
  • src/Tree.tsx (28 hunks)
  • src/TreeNode.tsx (2 hunks)
  • src/contextTypes.ts (3 hunks)
  • src/util.tsx (6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/Tree.tsx

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🪛 GitHub Check: codecov/patch
src/Tree.tsx

[warning] 757-757: src/Tree.tsx#L757
Added line #L757 was not covered by tests


[warning] 780-780: src/Tree.tsx#L780
Added line #L780 was not covered by tests


[warning] 882-882: src/Tree.tsx#L882
Added line #L882 was not covered by tests


[warning] 963-963: src/Tree.tsx#L963
Added line #L963 was not covered by tests

🔇 Additional comments (71)
src/TreeNode.tsx (52)

3-4: 导入模块的合理性检查
引入了React和TreeContext, UnstableContext,用于获取上下文与额外未稳定的上下文对象,看似用意清晰,可保持。无需额外改动。


17-39: 函数式组件重构
TreeNode 重构为函数式组件并使用 React.FC<Readonly<TreeNodeProps>>,有助于简化代码结构并更好地利用 Hooks。拆解 props 时请确保所有必需参数都符合组件需求。


41-41: 使用 React Context
TreeContext 获取上下文,这里能简化从父组件层层传递的繁琐度。


47-47: 本地状态 dragNodeHighlight
使用 useState 来跟踪拖拽高亮标记,命名语义直观,需要时也可搭配 useReducer 做更复杂的状态管理。


49-55: 可选属性逻辑
通过 useMemo 剪枝逻辑,将 selectable 或上下文的全局配置进行组合,性能与可维护性都得到保证。


57-59: 禁用节点判定
同样通过 useMemo 判断是否处于禁用状态,避免重复计算,逻辑清晰。


61-67: 可勾选性的动态获取
isCheckable 中,若树或节点本身不可勾选,就返回 false。可扩展性和可读性较好。


69-74: 点击选择事件
onSelect 中先判断是否禁用节点,逻辑简洁。调用 context.onNodeSelect 触发外部回调,符合组件解耦思路。


76-83: 点击勾选事件
若禁用或勾选被关闭则直接返回,否则交给上下文回调 onNodeCheck。此对外触发点有助于树全局管理。


86-93: 对选择与勾选进行区分
onSelectorClick 方法中先触发节点点击,再根据是否可选决定是调用 onSelect 还是 onCheck。逻辑清晰。


96-97: 双击事件
这里仅简单对外转发 context.onNodeDoubleClick,符合单一职责。


100-101: 鼠标进入事件
触发 onNodeMouseEnter,便于外部做 hover 效果或统计。实现合理。


104-105: 鼠标离开事件
调用 onNodeMouseLeave,与进入事件相对,应保持一致。


108-109: 右键菜单
onContextMenu 内直接转发事件,使用场景明确,暂无需改进。


112-120: 拖拽开始事件
开始时阻止冒泡并将 dragNodeHighlight 设为 true,还会尝试设置 dataTransfer。逻辑一致性良好,但请确认是否所有浏览器都需要此空字符串。


125-128: 拖拽进入事件
阻止默认事件,再调用 onNodeDragEnter。事件嵌套处理可靠。


131-134: 拖拽悬浮事件
与进入类似,同步阻止默认并调用 onNodeDragOver,使外部可处理显示样式或其他逻辑。


137-139: 拖拽离开事件
调用 context.onNodeDragLeave,方法的设计与其他拖拽事件保持一致。


142-145: 拖拽结束事件
取消高亮并调用 onNodeDragEnd,清理状态及时,符合拖拽尾期需求。


148-152: 节点被放下事件
onDrop 结束后也会将高亮设为 false,并调用 onNodeDrop,保证资源释放与事件同步可靠。


156-160: 延迟加载阻断
若节点处于loading状态则直接return,可防止重复展开或重复触发加载逻辑。


163-166: 子节点检测
hasChildren 基于 getEntity 的子节点是否存在来判断,逻辑简单易懂。


168-177: 计算叶子节点
memoizedIsLeaf 内针对各情况综合判断,减少重复计算并兼容异步加载场景。


179-184: 节点展开图标
nodeState 结合 expanded,返回 'open''close'或null,便于后续渲染。


187-195: 同步加载数据
若展开但未加载完成时会调用 onNodeLoad,避免重复加载,是正确的异步控制逻辑。


199-201: useEffect 同步加载
监听 expanded 和 loading 等依赖,在渲染后自动触发 syncLoadData,符合 Hook 最佳实践。


203-208: 可拖拽性判断
useMemo 根据 context.draggable 动态判断节点是否可拖拽,逻辑充分。


211-216: 渲染拖拽图标
dragHandlerNode 根据 draggable?.icon 进行条件渲染,符合可插拔设计。


219-223: 渲染开关图标函数
允许通过函数或JSX灵活定制,封装良好。


229-239: switcher 节点逻辑
针对叶子或非叶子处理不同的 switcher 展示,并绑定 onExpand,符合树形节点需求。


244-252: 非叶子分支
展开或收起时,会切换不同的 className 与图标,对于自定义交互具有可扩展性。


256-256: 空 switcher
switcherIcon 返回 false 时不渲染对应 span,避免多余 DOM。


258-262: checkbox 节点
isCheckable 为 false,直接返回 null,不浪费节点资源,合理。


265-283: checkbox 的自定义渲染
可支持自定义 element 并通过 className 区分选中、半选中、禁用状态,大大提高扩展性。


286-296: 默认图标 render
iconNode 根据节点状态加载与否变化 className,或可在此适配更多 loading 样式。


297-297: 空行或注释
无特别可补充意见,此处说明没有变动业务逻辑。


298-313: 渲染拖拽指示器
dropIndicatorNode 会根据上下文识别拖拽状态并调用 dropIndicatorRender,保持了顯示/隐藏逻辑的独立性。


314-322: 依赖收集
此处对 context 中的多个拖拽参数进行解构,以 useMemo 形式返回拖拽提示,性能与可维护性较好。


325-348: selectorNode 组合渲染
集中渲染标题、拖拽图标、icon等,使用 useMemo 提升性能,并在内部按需调用外部方法与自定义渲染参数。


349-349: loading状态 icon
showIcon 为真且 loading 为真时才使用原来的 loading icon,这里处理正确。


356-357: 自定义标题
title 为函数或全局 titleRender 都可灵活生成,能满足多种 UI 需求。


364-380: 包装内容多事件绑定
节点 wrapper 绑定鼠标事件、上下文菜单、双击单击等,为外部提供了大量监听点,可满足复杂交互需求。


381-395: useMemo 的依赖项
注意将和渲染相关的依赖都放进依赖列表中,以确保组件不会产生错误的缓存。


396-397: dataOrAriaAttributeProps
此处利用了 pickAttrsaria-data- 属性剥离,保证可访问性与数据属性能正确传递。


398-399: 获取节点层级
通过实体拿到 level 用于后续缩进与渲染逻辑。


400-401: 是否末尾节点
isEndNode 通过 isEnd 数组最后一个值判断,用于 leaf-last 类名。


402-406: 拖拽相关标识
isDraggabledragging 会配合 CSS 样式区分拖拽中的节点展现。


407-450: 整体容器渲染
使用 <div> 包裹所有子组件,并在上面挂载拖拽事件与 aria 属性,布局清晰、语义性合适。


451-451: TreeNode 标记
(TreeNode as any).isTreeNode = 1; 传统写法用于判定此组件是不是 TreeNode,对外提供类型识别。


452-452: 开发环境名称
在开发环境下为组件加 displayName,便于调试与错误边界分析。


455-455: 设置 displayName
仅在非生产环境时将组件名设置为 'TreeNode',方便调试和排查问题。


458-458: 默认导出
以默认方式导出 TreeNode 并与上文 displayName、type 标记相辅相成。

src/Tree.tsx (12)

31-31: 使用 TreeNodeProps 替换旧类型
TreeNodeProps 与数据结构更紧密,移除 NodeInstance 使类型命名更直观。


233-235: 类定义与类型参数
组件使用 class Tree<...> 并限制了 TreeDataType 范围,对类型安全性有益,但复杂度也稍高。


309-309: dragNodeProps 改为 TreeNodeProps
将原先的 dragNode 替换为 dragNodeProps,与新版结构统一。


421-421: flattenNodes 的更新
当 treeData 或 expandedKeys 改变时,会重新生成 flattenNodes,与新版逻辑保持一致。


Line range hint 440-447: 检查树的勾选逻辑
checkable 为真,会处理受控或非受控的勾选状态,并配合 conductCheck 完整收敛半选信息。


Line range hint 475-498: 拖拽开始事件
使用 (event, nodeProps),废弃了旧的 (event, node)。设置 dragNodeProps 并更新状态后再设置监听器,流程简明且可追踪。


Line range hint 508-614: 拖拽进入事件
onNodeDragEnter 中计算并设置可能的放置位置与展开逻辑,配合 set timeout 处理延时展开,细节完整。

🧰 Tools
🪛 Biome (1.9.4)

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)


Line range hint 619-695: 拖拽悬浮事件
通过 calcDropPosition 返回结果更新内部状态,若节点不可放置则不会更新 drag 状态。


Line range hint 698-711: 拖拽离开事件
利用 onNodeDragLeave 判断当前鼠标是否离开可放置区域,若是则重置拖拽状态,逻辑明确。


722-732: 拖拽结束事件
onNodeDragEnd 中也调用 cleanDragState 清除所有拖拽相关状态,避免残留影响。


Line range hint 737-790: 节点放下事件
onNodeDrop 中先检查 dropAllowed,若不允许则直接返回;否则将目标节点和拖拽节点打包后调用 onDrop。与外部对接明确。

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 757-757: src/Tree.tsx#L757
Added line #L757 was not covered by tests


757-757: 测试覆盖警告
新增或修改的行在单测中没有覆盖。若影响关键路径,建议补充测试用例。

Also applies to: 780-780, 882-882, 963-963

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 757-757: src/Tree.tsx#L757
Added line #L757 was not covered by tests

src/contextTypes.ts (3)

14-14: 类型统一
NodeInstance 替换为 TreeNodeProps,符合新版 TreeNode 架构。


40-40: 拖拽事件签名
NodeDragEventHandler 参数由 (e, nodeProps) 代替 (e, node),整合度提升,减少类型错配可能。


95-95: 新增上限类型给 TreeContext
React.createContext<TreeContextProps<any> | null> 明确了空初值和泛型类型,更易理解与维护。

src/util.tsx (4)

17-17: 类型替换
import {... TreeNodeProps} from './interface'; 代替旧有类型引用,统一类型定义。


88-89: 函数签名更新
calcDropPosition 将入参的 dragNodePropstargetNodeProps 都改为 TreeNodeProps,方便获取节点数据。


Line range hint 155-167: 校验可否放置
先判断是否位于第一个节点上方,再判断是否允许放置到此处。逻辑需要对比 allowDrop 返回值。


312-312: 解析勾选键值
keyProps 被显式类型标注 { checkedKeys?: Key[]; halfCheckedKeys?: Key[] },使后续处理更具可读性。

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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 (3)
src/TreeNode.tsx (2)

41-67: 建议优化 hooks 的性能表现

当前的 hooks 实现基本合理,但建议考虑以下优化:

  1. 可以将一些大型的渲染函数拆分为独立的组件
  2. 优化 useMemo 的依赖数组,避免不必要的重复计算

示例优化:

- const memoizedIsSelectable = React.useMemo<boolean>(() => {
-   if (typeof selectable === 'boolean') {
-     return selectable;
-   }
-   return context.selectable;
- }, [selectable, context.selectable]);
+ const memoizedIsSelectable = React.useMemo<boolean>(() => 
+   typeof selectable === 'boolean' ? selectable : context.selectable
+ , [selectable, context.selectable]);

406-449: 建议优化渲染性能

当前的渲染实现可以通过以下方式优化:

  1. 使用 useMemo 缓存复杂的 className 计算
  2. 将内联样式转换为 CSS 类
  3. 考虑使用 CSS-in-JS 方案统一管理样式

示例优化:

+ const nodeClassName = React.useMemo(() => 
+   classNames(className, `${context.prefixCls}-treenode`, {
+     [`${context.prefixCls}-treenode-disabled`]: isDisabled,
+     // ... other className calculations
+   }), 
+   [className, context.prefixCls, isDisabled]
+ );

  return (
    <div
-     className={classNames(className, `${context.prefixCls}-treenode`, {
-       [`${context.prefixCls}-treenode-disabled`]: isDisabled,
-       // ... other className calculations
-     })}
+     className={nodeClassName}
    >
src/Tree.tsx (1)

233-235: 建议加强类型系统的约束

当前的泛型类型定义可以通过以下方式改进:

  1. 为泛型添加更严格的约束
  2. 使用更具体的类型而不是联合类型
- class Tree<TreeDataType extends DataNode | BasicDataNode = DataNode | BasicDataNode>
+ class Tree<TreeDataType extends DataNode = DataNode>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0062e2b and f2128a5.

📒 Files selected for processing (2)
  • src/Tree.tsx (27 hunks)
  • src/TreeNode.tsx (2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/Tree.tsx

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🔇 Additional comments (2)
src/TreeNode.tsx (2)

17-39: 代码重构符合最新的React最佳实践!

将类组件重构为函数式组件的实现非常规范:

  • 使用 React.FC 类型声明
  • 使用 Readonly 确保 props 不可变性
  • 完整的 TypeScript 类型支持

112-152: 建议验证拖拽操作的边界情况

当前的拖拽实现需要注意以下几点:

  1. 建议增加对拖拽过程中可能出现的异常情况的处理
  2. 建议验证跨层级拖拽时的状态更新是否正确

Comment thread src/Tree.tsx
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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 (1)
src/Tree.tsx (1)

Line range hint 475-786: 拖拽处理函数重构保持了一致性

所有拖拽相关的事件处理函数都统一更新了参数类型,从 node 改为 nodeProps。这种一致性的改动有助于:

  • 代码更容易理解和维护
  • 类型检查更严格
  • 与组件重构方向保持一致

建议在代码注释中说明这些改动的目的,以帮助其他开发者理解重构的动机。

🧰 Tools
🪛 Biome (1.9.4)

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2128a5 and f128789.

📒 Files selected for processing (2)
  • src/Tree.tsx (18 hunks)
  • src/contextTypes.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/contextTypes.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/Tree.tsx

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🔇 Additional comments (2)
src/Tree.tsx (2)

309-309: 类型更改提升了类型安全性!

dragNode 重构为 dragNodeProps: TreeNodeProps<TreeDataType> 是一个很好的改进,它提供了更好的类型安全性和代码可维护性。


583-583: ⚠️ Potential issue

建议使用更安全的属性检查方法

当前代码使用 Object.hasOwnProperty 可能存在安全隐患,建议使用 Object.hasOwn() 替代:

- if (this.props.hasOwnProperty(name)) {
+ if (Object.hasOwn(this.props, name)) {

这样可以:

  • 避免原型链污染攻击
  • 提高代码的安全性
  • 符合现代 JavaScript 最佳实践

Likely invalid or redundant comment.

🧰 Tools
🪛 Biome (1.9.4)

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

Comment thread src/Tree.tsx Outdated
Comment thread src/TreeNode.tsx
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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/Tree.tsx (1)

需要继续进行类组件到函数组件的重构

根据验证结果,发现该组件仍然保留了大量类组件的特征:

  1. 仍然使用 class Tree extends React.Component 的写法
  2. 大量使用 this.statethis.setState 进行状态管理
  3. 使用类组件的生命周期方法如 componentDidMount
  4. 大量的实例方法绑定(如 this.onNodeClickthis.onNodeDragStart 等)

建议:

  • 将类组件转换为函数组件
  • 使用 useStateuseEffect 替代类组件的状态管理和生命周期
  • 使用 useCallback 优化事件处理函数
  • 使用 useRef 替代类的实例属性
🔗 Analysis chain

Line range hint 1-1439: 验证组件重构的完整性

请确保:

  1. 所有使用 NodeInstance 的地方都已更新为 TreeNodeProps
  2. 没有遗留的类组件特定的代码
  3. 性能没有因重构而降低

运行以下脚本来验证重构:


让我们进一步验证重构的完整性:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否还有 NodeInstance 的使用
rg "NodeInstance"

# 检查是否还有类组件特定的生命周期方法
ast-grep --pattern 'componentDidMount|componentDidUpdate|componentWillUnmount'

Length of output: 207


Script:

#!/bin/bash
# 检查是否还有其他类组件相关的代码
ast-grep --pattern 'class $_ extends React.Component'

# 检查是否有 this 的使用
rg "\bthis\." src/Tree.tsx

# 检查是否有状态管理相关代码
rg "setState|state =" src/Tree.tsx

Length of output: 9631

🧰 Tools
🪛 Biome (1.9.4)

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🧹 Nitpick comments (1)
src/Tree.tsx (1)

Line range hint 475-498: 建议优化拖拽处理逻辑

拖拽相关的事件处理函数有大量重复的状态处理逻辑,建议提取共用的状态更新逻辑到一个单独的辅助函数中。

例如 onNodeDragEnteronNodeDragOver 中有很多相似的状态更新代码。

建议重构为:

private updateDragState = (params: {
  dropPosition: number;
  dropLevelOffset: number;
  dropTargetKey: Key;
  dropContainerKey: Key;
  dropTargetPos: string;
  dropAllowed: boolean;
  dragOverNodeKey: Key;
}) => {
  this.setState(params);
};

这样可以简化事件处理函数,提高代码的可维护性。

Also applies to: 508-617

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f128789 and 2b95299.

📒 Files selected for processing (1)
  • src/Tree.tsx (18 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/Tree.tsx

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🔇 Additional comments (2)
src/Tree.tsx (2)

309-309: 类型重构改进了类型安全性!

dragNode 重构为 dragNodeProps: TreeNodeProps<TreeDataType> 是一个很好的改进,这与将 TreeNode 从类组件重构为函数组件的目标相符,同时提高了类型安全性。


583-583: 建议使用更安全的属性检查方法

当前代码使用 Object.hasOwnProperty 可能存在安全隐患,建议使用 Object.hasOwn() 替代。

- if (this.props.hasOwnProperty(name)) {
+ if (Object.hasOwn(this.props, name)) {
🧰 Tools
🪛 Biome (1.9.4)

[error] 583-583: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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/TreeNode.tsx (3)

41-216: 建议对 hooks 进行更清晰的分组

虽然代码已经有了一些注释分组,但建议进一步优化分组结构,例如:

  // ======= 基础 Hooks =======
  const context = React.useContext(TreeContext);
  const unstableContext = React.useContext(UnstableContext);
  const selectHandleRef = React.useRef<HTMLSpanElement>(null);
  
  // ======= 状态管理 Hooks =======
  const [dragNodeHighlight, setDragNodeHighlight] = React.useState<boolean>(false);
  
  // ======= 计算属性 Hooks =======
  const memoizedIsSelectable = React.useMemo(...);
  const isDisabled = React.useMemo(...);
  
  // ======= 业务逻辑 Hooks =======
  const isDraggable = React.useMemo(...);

这样的分组可以让代码结构更加清晰,提高可维护性。


72-156: 建议使用 useCallback 优化事件处理函数

当前的事件处理函数每次渲染都会重新创建,建议使用 useCallback 进行优化:

- const onSelect = (e: React.MouseEvent<HTMLSpanElement, MouseEvent>) => {
+ const onSelect = React.useCallback((e: React.MouseEvent<HTMLSpanElement, MouseEvent>) => {
    if (isDisabled) {
      return;
    }
    context.onNodeSelect(e, convertNodePropsToEventData(props));
-  };
+  }, [isDisabled, context.onNodeSelect, props]);

这样可以避免不必要的重渲染,提高性能。


333-457: 建议将复杂的渲染逻辑拆分成独立组件

当前的渲染逻辑较为复杂,建议将一些独立的部分抽取成单独的组件,比如:

+ const NodeIcon: React.FC<NodeIconProps> = ({ icon, loading, nodeState }) => {
+   return (
+     <span
+       className={classNames(
+         `${prefixCls}-iconEle`,
+         `${prefixCls}-icon__${nodeState || 'docu'}`,
+         { [`${prefixCls}-icon_loading`]: loading },
+       )}
+     />
+   );
+ };

+ const NodeTitle: React.FC<NodeTitleProps> = ({ title, data, titleRender }) => {
+   // 标题渲染逻辑
+ };

这样可以:

  1. 提高代码的可维护性
  2. 方便单元测试
  3. 提高代码的复用性
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b95299 and 461abdc.

📒 Files selected for processing (1)
  • src/TreeNode.tsx (2 hunks)
🔇 Additional comments (1)
src/TreeNode.tsx (1)

17-39: 代码结构清晰,使用了 Readonly 类型!

使用 Readonly 修饰符确保了 props 的不可变性,这是一个很好的实践。props 的解构也很清晰,便于维护。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants