fix: improve keybinding performance #4126
Conversation
|
/next |
Walkthrough该拉取请求对Monaco编辑器的命令和键绑定处理进行了多项修改。主要更新包括在 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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 and nitpick comments (2)
packages/monaco/src/common/command.ts (1)
7-8: 建议完善注释文档注释说明了常量的用途，但建议补充以下信息：
- 为什么这些特定命令需要保留默认快捷键
- 对性能的影响说明
建议更新注释为：
-// 不卸载 Monaco 内默认快捷键的命令白名单 +/** + * 保留 Monaco 编辑器默认快捷键的命令白名单。 + * 这些命令（如重命名相关操作）会保持其原有的快捷键绑定， + * 以确保基础编辑功能的正常运行，同时优化多实例场景下的性能表现。 + */packages/monaco/src/browser/monaco.contribution.ts (1)
599-607: 优化按键绑定注册逻辑通过跳过特定命令的按键绑定取消注册过程来提升性能。这个改动：
- 避免了不必要的按键绑定取消注册操作
- 保持了某些特定命令（如重命名）的原始按键绑定行为
这个改进与 PR 的性能优化目标完全一致。
建议考虑添加注释说明哪些命令会被跳过取消注册以及原因，这样可以帮助其他开发者更好地理解这个优化。
+ // Skip unregistering keybindings for specific commands (e.g. rename) to preserve their original behavior if (!SKIP_UNREGISTER_MONACO_KEYBINDINGS.includes(command)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/monaco/src/browser/monaco.contribution.ts (2 hunks)
- packages/monaco/src/browser/monaco.service.ts (0 hunks)
- packages/monaco/src/common/command.ts (1 hunks)
💤 Files with no reviewable changes (1)
- packages/monaco/src/browser/monaco.service.ts
🔇 Additional comments (2)
packages/monaco/src/common/command.ts (1)
8-8: 验证常量使用情况实现逻辑正确。建议验证此常量在其他模块中的使用情况，确保性能优化的效果。
packages/monaco/src/browser/monaco.contribution.ts (1)
82-82: 导入新的常量以优化按键绑定性能引入
SKIP_UNREGISTER_MONACO_KEYBINDINGS常量用于控制哪些命令可以跳过取消注册默认按键绑定的过程，这有助于提高性能。
|
🎉 PR Next publish successful! 3.4.5-next-1730102270.0 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v3.4 #4126 +/- ##
==========================================
+ Coverage 54.23% 54.25% +0.02%
==========================================
Files 1595 1598 +3
Lines 98029 97562 -467
Branches 20177 19947 -230
==========================================
- Hits 53169 52937 -232
+ Misses 37153 37073 -80
+ Partials 7707 7552 -155
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Types
Background or solution
之前是为了解决 rename 的输入框快捷键不展示的问题，而代理 monaco 的 lookupKeybinding 函数，但该函数在遇到非常多 monaco 的实例时（例如 notebook）会有性能问题。
本次修复将该 lookupKeybinding 去除，并使用其他方式解决 rename 的快捷键展示问题
Changelog
改进 keybinding 的性能问题
Summary by CodeRabbit
新特性
SKIP_UNREGISTER_MONACO_KEYBINDINGS，用于指定不应注销其默认键绑定的命令。变更
MonacoServiceImpl类中的方法进行了更新，部分方法已被弃用，建议使用新的服务注册方法。错误处理