Skip to content

fix(wizard): 修复工具菜单上下键需先按回车才能响应#25

Open
S0ngRu1 wants to merge 1 commit into
qiniu:mainfrom
S0ngRu1:fix/menu-keyboard-unresponsive
Open

fix(wizard): 修复工具菜单上下键需先按回车才能响应#25
S0ngRu1 wants to merge 1 commit into
qiniu:mainfrom
S0ngRu1:fix/menu-keyboard-unresponsive

Conversation

@S0ngRu1
Copy link
Copy Markdown

@S0ngRu1 S0ngRu1 commented May 25, 2026

Description

修复 issue #17 — 工具菜单中进入任一子流程后返回,上下键无响应,必须先按回车才能选择。

根因

项目混用了两套 inquirer:

  • tool-menu.ts 使用 @inquirer/select(modular API)
  • promptHelper 使用老 inquirer 9.x(数组式 API)

迁移到 @inquirer/* 后又发现 @inquirer/core 存在两个不兼容副本:

  • @inquirer/select@4.x @inquirer/search@3.xcore@10.3.2
  • @inquirer/input @inquirer/password @inquirer/confirm @inquirer/checkboxcore@11.1.10

两个 core 各自接管 stdin 的 readline 实例。从 input(core@11) 返回后,下一次 select(core@10) 启动时 core@10 重新接管 stdin,但 core@11 残留的 keypress 监听器还挂着,按键被吞——按回车触发某个事件让残留监听器收尾,stdin 才稳定下来。

额外发现:fetchLatestVersion 网络失败返回 null 时,tool-menu.ts 的 abort 钩子会反复触发,造成 select 死循环重渲染(issue #17 原帖描述的核心现象)。

Changes

  • 卸载 inquirer ^9.2.12@types/inquirer,全部 prompt 改用 @inquirer/* modular API(input/password/confirm/checkbox
  • 升级 @inquirer/select5.1.5@inquirer/search4.1.9,让所有 @inquirer/* 共享 @inquirer/core@11.1.10,消除 stdin readline 接管冲突
  • showToolMenu 中加入 versionFetched 标志位(与 latestVersion 同一回调中赋值,保证早于 abort 钩子触发),避免 fetchPromise 已 resolved 时反复触发 abort
  • renderVersionInfo 新增 fetched 参数:查询完成但未拿到版本时显示 "未能获取(请检查网络)" 而不是一直显示 "检查中..."
  • 新增 i18n 文案 tool_version_unavailable(zh_CN / en_US)

Checklist

  • `pnpm build` succeeds
  • `node dist/cli.js --version` outputs correct version
  • Tested basic CLI functionality
  • Updated translations if adding user-facing strings

将 promptHelper 从老 inquirer 9.x 迁移到 @inquirer/* modular API,
并升级 @inquirer/select、@inquirer/search 到 5.x/4.x,让所有
@inquirer/* 子包共享同一份 @inquirer/core@11,消除 stdin readline
残留监听器导致的按键被吞问题(issue qiniu#17)。

同时在 showToolMenu 中加入 versionFetched 标志位,避免 npm registry
不可达时 fetchPromise 反复触发 abort,造成 select 死循环。

closes qiniu#17
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the project from the monolithic inquirer package to modular @inquirer/* packages to resolve input issues such as unresponsive arrow keys. It also fixes a potential infinite loop in the tool menu caused by improper handling of failed version fetches and adds localized "unavailable" status messages. Review feedback suggests refining the promptHelper by relaxing generic constraints, removing redundant mapping logic, adding AbortSignal support for better integration, and removing the unused @inquirer/confirm dependency.

Comment on lines 17 to 22
async select<T extends string>(message: string, choices: { name: string; value: T }[]): Promise<T> {
const { answer } = await inquirer.prompt([
{
type: 'list',
name: 'answer',
message,
choices,
},
]);
return answer as T;
return await select<T>({
message,
choices: choices.map(c => ({ name: c.name, value: c.value })),
});
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

泛型约束 T extends string 过于严格,且 choices.map 调用是多余的。底层的 @inquirer/select 支持任何类型的值(如布尔值、数字等)。此外,建议增加对可选 AbortSignal 的支持,这样该 helper 就能在 tool-menu.ts 的自动刷新逻辑中被复用,提高代码一致性。

Suggested change
async select<T extends string>(message: string, choices: { name: string; value: T }[]): Promise<T> {
const { answer } = await inquirer.prompt([
{
type: 'list',
name: 'answer',
message,
choices,
},
]);
return answer as T;
return await select<T>({
message,
choices: choices.map(c => ({ name: c.name, value: c.value })),
});
},
async select<T>(message: string, choices: { name: string; value: T }[], signal?: AbortSignal): Promise<T> {
return await select<T>({
message,
choices,
}, { signal });
},

Comment on lines 55 to 61
async checkbox<T extends string>(message: string, choices: { name: string; value: T }[]): Promise<T[]> {
const { answer } = await inquirer.prompt([
{
type: 'checkbox',
name: 'answer',
message,
choices,
pageSize: 15,
},
]);
return answer as T[];
return await checkbox<T>({
message,
choices: choices.map(c => ({ name: c.name, value: c.value })),
pageSize: 15,
});
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

select 方法类似,建议将 checkbox 改为泛型并支持 AbortSignal,同时移除多余的 choices.map 操作。

Suggested change
async checkbox<T extends string>(message: string, choices: { name: string; value: T }[]): Promise<T[]> {
const { answer } = await inquirer.prompt([
{
type: 'checkbox',
name: 'answer',
message,
choices,
pageSize: 15,
},
]);
return answer as T[];
return await checkbox<T>({
message,
choices: choices.map(c => ({ name: c.name, value: c.value })),
pageSize: 15,
});
},
async checkbox<T>(message: string, choices: { name: string; value: T }[], signal?: AbortSignal): Promise<T[]> {
return await checkbox<T>({
message,
choices,
pageSize: 15,
}, { signal });
},

Comment thread package.json
"@inquirer/search": "^3.2.2",
"@inquirer/select": "^4.2.1",
"@inquirer/checkbox": "^5.1.5",
"@inquirer/confirm": "^6.0.13",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

依赖项中新增了 @inquirer/confirm,但在代码中并未实际使用。promptHelper.confirm 目前是基于 @inquirer/select 实现的列表式确认。如果确定不需要标准的 (y/N) 确认框,建议移除此依赖以保持 package 简洁。

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. I found one small dependency cleanup item; security and documentation passes found no noteworthy issues.

Comment thread package.json
"@inquirer/search": "^3.2.2",
"@inquirer/select": "^4.2.1",
"@inquirer/checkbox": "^5.1.5",
"@inquirer/confirm": "^6.0.13",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: @inquirer/confirm is added as a production dependency but the migrated helper still implements confirm() with @inquirer/select, and there are no imports of @inquirer/confirm elsewhere. Please drop this dependency (and the corresponding lockfile entries) or switch promptHelper.confirm to use it, otherwise installs include an unused package.

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.

1 participant