Repository navigation
fix(@inquirer/core): clear hook effects before settling prompts - #2255
Merged
SBoudrias merged 2 commits intoSep 7, 2026
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2255 +/- ##
=======================================
Coverage ? 89.45%
=======================================
Files ? 48
Lines ? 2266
Branches ? 602
=======================================
Hits ? 2027
Misses ? 230
Partials ? 9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
SBoudrias
force-pushed
the
fix-core-hook-cleanup-settlement
branch
from
September 7, 2026 16:48
ccde777 to
6d6d3b8
Compare
Characterize how a prompt settles when a useEffect cleanup throws: the prompt rejects with the cleanup error, and the answer (or the original settlement error, e.g. AbortPromptError) is dropped. These tests pass on current main; they guard the existing settlement semantics against regressions while the settlement path is reworked in the next commit.
SBoudrias
force-pushed
the
fix-core-hook-cleanup-settlement
branch
3 times, most recently
from
September 7, 2026 17:32
4f26d91 to
7518e0a
Compare
createPrompt cleared hook effects only after the prompt promise settled, in a microtask. But readline emits keypresses synchronously within a single data event, so input batched with the key that settled the prompt still reached useKeypress handlers before cleanup ran. Clear hook effects in the synchronous settlement path instead: - resolve: clear effects, then resolve - reject/cancel/abort: clear effects, then reject The interface cleanup (screen.done + output.end) still runs in finally, so it happens after the prompt reaches its done state. As a side effect, prompts created with an already-aborted signal now also run the terminal cleanup instead of leaking the readline interface. The settlement path is kept minimal: the promise resolvers and effectScheduler.clearAll() are both idempotent, so no settled guard is needed; the bound cleanup is named clearEffects to avoid shadowing the hook-engine store field name. Event listener registrations go through a listenTo() helper returning the disposer, so each site collapses into a single cleanups.add(listenTo(...)) statement. Adds regression coverage for the done, abort, cancel, and already-aborted signal settlement paths. Closes SBoudrias#1816 Co-authored-by: Joel Towell <156634308+JoelTowell@users.noreply.github.com>
SBoudrias
force-pushed
the
fix-core-hook-cleanup-settlement
branch
from
September 7, 2026 17:37
7518e0a to
67e6fe2
Compare
SBoudrias
approved these changes
Sep 7, 2026
SBoudrias
left a comment
Owner
There was a problem hiding this comment.
Thanks! This is a very clean PR! 👏🏻
1 task
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Closes #1816.
createPromptcurrently clears hook effects after the prompt promise has already settled. That means listeners installed by hooks can still handle input emitted in the same tick afterdone()or after the prompt is aborted.This moves hook cleanup into the shared settlement path instead:
The final interface cleanup still stays in
finally, soscreen.done()andoutput.end()continue to run after the prompt reaches its done state.I added regression coverage for both cases: settling from a
useKeypresshandler, and aborting the prompt.Commands run
corepack yarn pretestcorepack yarn vitest --run packages/core/core.test.ts