fix(tabbar): fix missing keybindingRegistry dependency in IconTabView - #4619
HellowVirgil wants to merge 1 commit into
Conversation
|
|
Walkthrough将 IconTabView 中用于计算标题的 useMemo 增加空值保护并将依赖从 Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 分钟 Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
packages/main-layout/src/browser/tabbar/bar.view.tsx (3)
147-150: 避免在渲染阶段产生副作用：把 updateTabInMoreKey(true) 移入 useEffect在 render 期间执行副作用（更新 service 状态）会带来难以预测的重复调用与顺序问题，且违反 React 约定。建议将其迁移到 useEffect，并以 hideContainers 为依赖。
建议改动（删除渲染期副作用）：
- hideContainers.forEach((componentInfo) => { - tabbarService.updateTabInMoreKey(componentInfo.options!.containerId, true); - }); + // moved to useEffect to avoid side-effects during render在组件内补充一个 effect（在合适位置添加，不必紧邻本行）：
useEffect(() => { hideContainers.forEach((c) => { tabbarService.updateTabInMoreKey(c.options!.containerId, true); }); visibleContainers.forEach((c) => { tabbarService.updateTabInMoreKey(c.options!.containerId, false); }); }, [hideContainers, visibleContainers, tabbarService]);
151-212: useCallback 依赖缺失导致闭包值陈旧renderContainers 使用了 TabView、tabbarService、tabClassName、willHideTabbar、forbidCollapse 等外部变量，但依赖数组为空，可能导致点击/拖拽逻辑使用过期值（例如 willHideTabbar 或 forbidCollapse 变化后不生效）。
建议改动：
- const renderContainers = useCallback( + const renderContainers = useCallback( (component: ComponentRegistryInfo, tabbarService: TabbarService, currentContainerId?: string, side?: string) => { // ... }, - [], + [TabView, tabbarService, tabClassName, willHideTabbar, forbidCollapse], );
158-159: 同样避免在渲染阶段更新 service 状态这里对可见项执行 updateTabInMoreKey(false) 也属于副作用，建议移除并交由上文建议的 useEffect 统一处理。
建议改动：
- tabbarService.updateTabInMoreKey(containerId, false); + // moved to useEffect to avoid side-effects during render
🧹 Nitpick comments (1)
packages/main-layout/src/browser/tabbar/bar.view.tsx (1)
301-309: 拼写小问题：Elipses → Ellipses纯展示组件命名建议修正拼写（不影响功能，但可读性更佳）。若对外有引用，请同步更新引用点。
建议改动（节选）：
-export const IconElipses: FC = () => { +export const IconEllipses: FC = () => { // ... -export const TextElipses: FC = () => ( +export const TextEllipses: FC = () => (引用处：
- MoreTabView={IconElipses} + MoreTabView={IconEllipses} - MoreTabView={IconElipses} + MoreTabView={IconEllipses} - MoreTabView={TextElipses} + MoreTabView={TextEllipses} - MoreTabView={IconElipses} + MoreTabView={IconEllipses}Also applies to: 311-318, 336-336, 381-381, 410-410, 432-432
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/main-layout/src/browser/tabbar/bar.view.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/main-layout/src/browser/tabbar/bar.view.tsx (1)
packages/ai-native/src/browser/contrib/intelligent-completions/intelligent-completions.controller.ts (1)
keybindingRegistry(71-73)
🔇 Additional comments (2)
packages/main-layout/src/browser/tabbar/bar.view.tsx (2)
248-255: 修复正确：为 useMemo 补充 keybindingRegistry 依赖，避免崩溃与标题不刷新问题该改动使标题在 keybindingRegistry 变更时能正确重新计算，同时避免 React 比较依赖时的异常。变更粒度小且定位精准，赞。
248-255: 确认 – 未发现其他缺失的 keybindingRegistry 依赖
已在全仓库范围内扫描所有 useMemo/useCallback 回调体中对 keybindingRegistry 的引用，未检测到未在依赖数组中声明的场景，可放心忽略此项风险。
c5a61d9 to
0cac378
Compare
|
/next |
|
🎉 PR Next publish successful! 3.9.1-next-1755164421.0 |
3 participants
Types
Background
bar.view.tsxthrow errorUncaught TypeError: Cannot read properties of undefined.Cause
Solution
Add keybindingRegistry to the dependency array of useMemo.
Summary by CodeRabbit