-
Notifications
You must be signed in to change notification settings - Fork 310
【高优先】重构 GM_registerMenuCommand 设计:稳定 menu item ID、同步更新、跨 iframe 一致性,行为对齐 Tampermonkey
#820
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
Conversation
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.
Pull Request Overview
本 PR 重构了 GM_registerMenuCommand 的设计以解决与 Tampermonkey 的行为差异问题,实现了稳定的菜单项 ID、即时同步更新、跨 iframe 一致性等功能。
- 将菜单项 ID 从数字改为基于内容的稳定唯一键(groupKey + contentEnvKey)
- 实现了即时菜单更新机制,无需关闭再打开 popup
- 统一了主框架和子框架之间的菜单显示与触发逻辑
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/store/AppContext.tsx | 增加类型定义和泛型支持,优化消息订阅机制 |
| src/pages/popup/App.tsx | 添加实时菜单更新订阅和排序逻辑,支持动态菜单变更 |
| src/pages/options/routes/Setting.tsx | 统一消息订阅的类型定义 |
| src/pages/options/routes/ScriptList/index.tsx | 统一消息订阅的类型定义 |
| src/pages/components/ScriptMenuList/index.tsx | 重构菜单项分组逻辑,实现 groupKey 合并显示 |
| src/app/service/service_worker/types.ts | 新增菜单相关类型定义,支持新的菜单项结构 |
| src/app/service/service_worker/popup.ts | 核心重构:实现稳定 contextMenu ID 和菜单同步机制 |
| src/app/service/service_worker/gm_api.ts | 更新 GM API 参数类型,适配新的菜单键结构 |
| src/app/service/service_worker/client.ts | 更新客户端接口,支持新的菜单参数结构 |
| src/app/service/queue.ts | 更新队列类型定义,支持新的菜单项键结构 |
| src/app/service/content/gm_api.ts | 重构客户端菜单注册逻辑,实现环境隔离和键管理 |
| src/app/service/content/gm_api.test.ts | 添加菜单注册和取消注册的测试用例 |
| src/app/cache.ts | 添加注释说明缓存返回值的边界情况 |
Comments suppressed due to low confidence (1)
src/app/service/service_worker/popup.ts:1
- 使用非空断言操作符
!可能导致运行时错误。应该检查data是否存在以及tabId是否为有效键,例如return data?.[sender.getExtMessageSender().tabId] || {}。
import { type IMessageQueue } from "@Packages/message/message_queue";
| ]; | ||
| return () => { | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
Copilot
AI
Oct 9, 2025
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.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| //@ts-ignore | ||
| b.enable - a.enable || |
Copilot
AI
Oct 9, 2025
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.
使用 @ts-ignore 忽略类型检查不是好的做法。应该通过类型断言或修复类型定义来解决类型错误,例如 Number(b.enable) - Number(a.enable)。
| //@ts-ignore | |
| b.enable - a.enable || | |
| Number(b.enable) - Number(a.enable) || |
| return () => { | ||
| isMounted = false; | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
Copilot
AI
Oct 9, 2025
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.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| ]; | ||
| return () => { | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
Copilot
AI
Oct 9, 2025
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.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| ]; | ||
| return () => { | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
Copilot
AI
Oct 9, 2025
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.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| return () => { | ||
| isMounted = false; | ||
| unsub(); | ||
| checkItems.clear(); |
Copilot
AI
Oct 9, 2025
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.
在 useEffect 清理函数中调用 checkItems.clear() 是多余的。Map 实例会在组件卸载后自动被垃圾回收,无需手动清理。
| checkItems.clear(); |
| // @ts-ignore | ||
| const exec = new ExecScript(script, "content", mockMessage, nilFn, envInfo); |
Copilot
AI
Oct 9, 2025
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.
在测试代码中使用 @ts-ignore 并不理想。应该通过正确的类型定义或类型断言来解决类型问题,例如使用 as any 或创建合适的 mock 类型。
| // @ts-ignore | ||
| const exec = new ExecScript(script, "content", mockMessage, nilFn, envInfo); |
Copilot
AI
Oct 9, 2025
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.
在测试代码中使用 @ts-ignore 并不理想。应该通过正确的类型定义或类型断言来解决类型问题,例如使用 as any 或创建合适的 mock 类型。
| // @ts-ignore | |
| const exec = new ExecScript(script, "content", mockMessage, nilFn, envInfo); | |
| const exec = new ExecScript(script, "content", mockMessage as any, nilFn, envInfo); |
|
似乎也没有太多有营养的内容,和之前差不多 |
|
Copilot Review完 |
详细见 #790 . 这PR只用来给Copilot Review
背景 / 动机
现有
GM_registerMenuCommand的行为与 Tampermonkey(TM)存在差异,导致:interval-n-xxxx)不显示。id的注册覆盖规则与期望不一致。本 PR 针对上述问题进行全面修订,使行为与 TM 对齐,并关闭 #787。
变更重点
稳定化浏览器 contextMenu 的项目 ID
interval-n-xxxx)。即时与可预期的动态更新
intervalChanging = true)时,popup 与 context menu 会即时反映,无需关闭再开。行为与 TM 一致。跨 frame(top 与 iframe)的一致性
interval-m-1001与interval-m-1002在不同 frame 被错误合并/拆分),现在结果与 TM 一致。相同
id的覆盖规则id重复注册时,只保留最新一个(举例:「MenuReg abc-2」覆盖「MenuReg abc-1」),并确保GM_unregisterMenuCommand(id)能正确移除对应项目。accessKey与返回值一致性accessKey更新与同一id下的覆盖;注册与反注册的返回值/语意保持一致。实作概要
GM_registerMenuCommand与GM_unregisterMenuCommand的内部资料结构与索引策略,确保 稳定 ID、单一事实来源 与 原子更新。id注册视为更新;解除注册支援以id、或以注册返回值识别。相容性 / 风险
id的脚本行为不变;多次注册同id的脚本现在会得到 TM 一致的覆盖效果。id多重并存」的非预期行为,升级后可能只看到最后一次注册的项目。建议在 release note 提醒。测试方案(可直接复制以下脚本验证)
将以下 Userscript 于任意页面执行,并依照注解切换三个测试开关:
checkSubFrameIdSequence:在有 iframe 的页面设为true,检查 top 与 iframe 的 ID/顺序。intervalChanging:设为true检查「每秒动态变更 menu」是否即时更新。skipClickCheck:预设false会引导互动测试;设true可跳过引导。此脚本已内嵌于 PR 讨论中(详见原 PR 内容)。
预期结果(关键观察点):
id注册仅显示最新一个条目(abc→ 只剩MenuReg abc-2)。id注册会同时显示两个条目,新增带accessKey的版本后,三者并存规则正确。GM_unregisterMenuCommand逐一移除后,对应条目正确消失。intervalChanging = true时,popup 与右键 context menu 即时刷新,不需重开。效能 / 可靠性
相关议题
GM_registerMenuCommand问题 #787检查清单
id覆盖 / 跨 frame)原PR: #790 "[高优先] 重新修订
GM_registerMenuCommand相关代码设计 by cyfung1031 · Pull Request #790 · scriptscat/scriptcat · GitHub"