Repository navigation
fix(cli): repair the live slash gate fallout on main #10940
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a075310
test(cli): give the mock memory command an action
yiliang114 1e5dd89
fix(cli): add the missing slashCommands dep to the keypress callback
yiliang114 9cbd2e1
test(cli): note why the mock memory command needs an action
yiliang114 6fd9daf
Merge branch 'main' into autofix/issue-10935
yiliang114 4076bc7
Merge branch 'main' into autofix/issue-10935
qwen-code-dev-bot 043681b
Merge branch 'main' into autofix/issue-10935
qwen-code-dev-bot e097ba8
Merge branch 'main' into autofix/issue-10935
qwen-code-dev-bot b0def7e
Merge branch 'main' into autofix/issue-10935
qwen-code-dev-bot c447c8c
Merge branch 'main' into autofix/issue-10935
qwen-code-dev-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Declined — the requested test cannot exist, because the stale-closure state it would pin is unreachable.
exportCompletionis also a dependency of this sameuseCallback, and its identity is derived fromslashCommands:useExportCompletioncomputesexportFormatSuggestions = useMemo(..., [slashCommands]), which flows intoexportCycleFormats, then intohandleExportInputandsuggestionDisplayProps, then into the memoizedexportCompletion. So any change to theslashCommandsprop identity already recreates the keypress callback and captures the current list, whether or not the explicit entry is there.I built the test you sketched (render with
memoryfiltered out,rerenderwith the full list, Enter on a buffered/memory) and ran your acceptance criterion:Instrumenting the gate at Enter time, with the dep deleted, shows why:
Mutating the array in place is not an alternative either: a stale closure over the same array object reads the mutated content, and
Object.iswould not fire for the explicit dep either way.So there is no input where deleting the dep changes behavior, and
react-hooks/exhaustive-depsunder CI's--max-warnings 0remains the operative guard, exactly as your witness showed. The dep itself stays: it is lint-required and correct on its own terms.One caveat in your favour, recorded so it is not lost: the redundancy is indirect. It holds only while
useExportCompletionkeeps deriving fromslashCommands. If that derivation is ever removed, the explicit entry becomes load-bearing and lint would again be the only guard. That is a reason to keep the dep, not a reason to add a test that asserts nothing — so I dropped the drafted test rather than ship 40 lines that are green either way.中文说明
不予采纳 —— 所要求的测试无法存在,因为它想钉住的"过期闭包"状态是不可达的。
exportCompletion同样是这个useCallback的依赖,而它的 identity 是由slashCommands派生出来的:useExportCompletion计算exportFormatSuggestions = useMemo(..., [slashCommands]),它流向exportCycleFormats,再流向handleExportInput与suggestionDisplayProps,最后进入被 memo 的exportCompletion。因此只要slashCommands这个 prop 的 identity 发生变化,按键回调就已经会被重建并捕获最新的命令列表——无论显式依赖项在不在。我按你的草图写了这个测试(先用过滤掉
memory的列表渲染,再用完整列表rerender,然后在缓冲区已有/memory的情况下按 Enter),并执行了你给出的验收标准:在删除依赖项的情况下对 Enter 时的判定打点,说明了原因:
"原地修改数组"也不是另一条路:对同一个数组对象的过期闭包会读到被修改后的内容,而
Object.is在两种情况下都不会为显式依赖项触发。所以不存在任何输入能让"删除该依赖项"改变行为,而在 CI 的
--max-warnings 0下,react-hooks/exhaustive-deps仍然是真正起作用的守卫——这与你的 witness 结论一致。依赖项本身保留:它是 lint 所要求的,就其自身而言也是正确的。有一点对你有利,记录下来以免丢失:这种冗余是间接的。它只在
useExportCompletion继续从slashCommands派生时成立。一旦这个派生关系被移除,显式依赖项就会变成关键项,而 lint 又将再次成为唯一的守卫。这是保留该依赖项的理由,而不是新增一个什么都断言不了的测试的理由——因此我删掉了已起草的测试,而不是提交 40 行两种情况下都绿的代码。