Skip to content

fix(file-tree): 修复切换仓库后目录树展开状态丢失的问题#338

Merged
J3n5en merged 2 commits intoJ3n5en:mainfrom
lwt-sadais:fix/file-tree-expanded-state-loss
Mar 5, 2026
Merged

fix(file-tree): 修复切换仓库后目录树展开状态丢失的问题#338
J3n5en merged 2 commits intoJ3n5en:mainfrom
lwt-sadais:fix/file-tree-expanded-state-loss

Conversation

@lwt-sadais
Copy link
Contributor

问题描述

在目录树中展开多层文件夹后,收起父节点(或某个祖先节点),切换到其他仓库再切换回来,目录树的展开状态丢失。

具体表现:

  • 收起有多个子目录的父节点后,子节点路径残留在 expandedPathslocalStorage
  • 切换仓库回来时,restoreChildren 尝试恢复这些孤立路径,但因父节点未展开而静默失败
  • 用户重新打开父节点时,子节点显示"展开"箭头但内容为空

根因分析

collectCompactedPaths 仅处理压缩链(单个子目录的连续路径):当父节点有多个子目录时,只删除父节点路径,子节点路径作为孤立路径残留在 localStorage 中。

折叠 /a(有多个子目录 /a/b、/a/c)
→ 仅删除 /a
→ /a/b 残留在 expandedPaths
→ 切换仓库回来时,/a/b 在树中找不到 → 静默失败
→ 展开 /a 后,/a/b 显示箭头但无内容

修复方案

将折叠逻辑从"仅移除 compact chain 路径"改为移除目标路径及其所有后代展开路径

// Before: only removes compact chain paths
const pathsToCollapse = collectCompactedPaths(treeRef.current, path);
for (const p of pathsToCollapse) {
  newExpanded.delete(p);
}

// After: remove target and all expanded descendants
for (const p of [...newExpanded]) {
  if (p === path || p.startsWith(`${path}/`)) {
    newExpanded.delete(p);
  }
}

行为对比

场景 修复前 修复后
折叠有多个子目录的父节点 ❌ 子路径残留 localStorage ✅ 全部清除
折叠 compact chain(单子目录链) ✅ 沿链删除 ✅ 同样全部清除(等价)
切换仓库再切回后重新展开父节点 ❌ 子节点显示箭头但无内容 ✅ 子节点正常折叠,可重新展开

代码变更

  • 移除 collectCompactedPaths 递归函数(29 行)
  • 替换为简单的集合遍历(6 行)
  • 行为覆盖范围更广,逻辑更清晰

🤖 Generated with Claude Code

修复折叠逻辑,当用户折叠一个包含多个子目录的父节点时,现在会同时清除该目录及其所有展开的后代路径。这防止了孤立的子路径残留在 localStorage 中,导致用户在切换仓库回来后看到子节点显示为"展开"状态但内容为空的现象。

变更:
- 简化折叠逻辑:移除复杂的 collectCompactedPaths 递归函数
- 改为直接清除目标路径及所有以其开头的后代路径
- 此行为对压缩链(single-child directories)仍然有效,因为它们本质上就是后代关系

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2026

Claude finished @lwt-sadais's task in 1m 18s —— View job


🔍 代码审查报告

👋 欢迎查看审查结果! 本次审查已完成,以下是详细分析和建议。
💡 使用提示:点击 📁 文件链接可直接跳转到代码位置,建议按优先级处理问题。

📊 审查概览 · 一目了然

📋 审查项目 🔢 发现数量 📈 占比 🎯 处理状态
📁 审查文件 1 100% 已完成
🚨 严重问题 0 0%
⚠️ 中等问题 0 0%
轻微问题 1 100% 🔵 后续优化

🏆 代码质量评分

📊 综合评分: 92/100  🌟🌟🌟🌟⭐
📋 本次等级: 优秀

📋 问题分布

🚨 严重: ░░░░░░░░░░ 0%
⚠️ 中等: ░░░░░░░░░░ 0%
⚡ 轻微: █░░░░░░░░░ 10%
总体评分: ⭐⭐⭐⭐⭐ (5/5)

🎯 变更分析

本次 PR 修改了 src/renderer/hooks/useFileTree.ts,核心变更是将 toggleExpand 函数中折叠目录时的逻辑从递归遍历树结构改为直接遍历 expandedPaths 集合。

变更要点:

  • 删除 collectCompactedPaths 递归函数(29 行),该函数仅处理 compact chain(单子目录链),无法清除多子目录场景下的后代路径
  • 替换为 简洁的集合遍历(6 行),移除目标路径及其所有后代展开路径

修复效果:

- // Before: collectCompactedPaths 只沿单子目录链向下收集
- // 多子目录场景下 /a/b、/a/c 等路径残留在 expandedPaths
+ // After: 直接遍历 expandedPaths,删除所有以 path/ 开头的后代
+ for (const p of [...newExpanded]) {
+   if (p === path || p.startsWith(`${path}/`)) {
+     newExpanded.delete(p);
+   }
+ }

✅ 审查亮点

  1. 根因分析准确 — PR 描述清晰地定位了 collectCompactedPaths 仅处理 compact chain 的局限性,导致多子目录场景下子路径残留
  2. 修复方案优雅 — 新方案不依赖树结构遍历,直接基于路径前缀匹配,逻辑更简洁且覆盖范围更广
  3. 代码减少 23 行 — 删除了复杂的递归逻辑,降低了维护成本
  4. 注释使用英文 — 符合项目 CLAUDE.md 中的代码注释规范
  5. refreshNodeChildren 一致 — 在 useFileTree.ts:304-306 中删除目录时也使用了相同的前缀匹配模式,保持了代码一致性

🐛 问题清单与修复指南

📋 👆 点击此处展开详细问题列表 (共 1 个问题) · 一键复制追踪 📌

⚡ 轻微级别 (Minor) 优化建议

🔵 #1 src/renderer/hooks/useFileTree.ts:170

🏷️ 类型: 性能微优化 | ⏰ 优先级: P2 - 后续版本 | 🎯 影响范围: 局部

  • 🔍 问题描述:当前使用 [...newExpanded] 展开为数组再遍历,在大型项目中展开路径较多时会有额外的内存分配。可以直接遍历 Set 本身,因为 Set.prototype.delete 在遍历过程中是安全的(已删除的元素不会被重复访问,但新增元素的行为未定义 — 此处无新增操作)。
  • 🛠️ 解决方案
    // src/renderer/hooks/useFileTree.ts:170-174
    // 可选优化:直接遍历 Set,避免数组拷贝
    for (const p of newExpanded) {
      if (p === path || p.startsWith(`${path}/`)) {
        newExpanded.delete(p);
      }
    }
  • ⚠️ 潜在影响:影响极小,仅为性能微优化。当前实现已经完全正确,展开数组的方式更保守安全。不建议为此阻塞合并。

📊 修复优先级矩阵

🎯 优先级 🚨 严重 ⚠️ 中等 ⚡ 轻微 📈 总计
🔴 P0 0 个 - - 0 个
🟡 P1 - 0 个 - 0 个
🔵 P2 - - 1 个 1 个
📊 合计 0 0 1 1 个

🎯 合并决策建议

📋 合并评估结果

📊 评估维度 ⭐ 得分 🎯 状态 💭 说明
🔐 安全性 10/10 ✅ 通过 无安全风险,纯前端状态管理逻辑
📊 代码质量 9/10 ✅ 优秀 代码简洁,逻辑清晰,注释规范
⚡ 性能影响 9/10 ✅ 无影响 路径前缀匹配复杂度合理
🛠️ 功能完整性 10/10 ✅ 完整 覆盖多子目录和 compact chain 两种场景
🧪 测试覆盖 7/10 ⚠️ 基本 无自动化测试,但变更属于 bug 修复且逻辑简单

📈 综合评分: 45/50 分 · 等级: 优秀


🚦 最终建议

建议合并

📝 详细理由

  • 🟢 根因分析准确,修复方案正确且覆盖范围更广
  • 🟢 代码量净减少 23 行,复杂度显著降低
  • 🟢 与项目中已有的路径清理模式(refreshNodeChildren)保持一致
  • 🟢 无安全风险,无性能退化
  • 🟢 注释使用英文,符合项目规范

🎯 后续版本可选改进

  1. 可选
    • 考虑直接遍历 Set 替代展开为数组(P2,微优化)

@J3n5en J3n5en merged commit bde560f into J3n5en:main Mar 5, 2026
1 check passed
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