Skip to content

Commit

Permalink
Fix(@inquirer/core) Clean hooks immediately on close events. (#1486)
Browse files Browse the repository at this point in the history
Fix(@inquirer/select) Clear remaining timeouts on exit

---------

Co-authored-by: Simon Boudrias <[email protected]>
  • Loading branch information
matteosacchetto and SBoudrias authored Jul 30, 2024
1 parent b89972b commit 3610c68
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 2 deletions.
40 changes: 40 additions & 0 deletions packages/core/core.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,46 @@ describe('createPrompt()', () => {
await expect(answer).resolves.toEqual('done');
expect(getScreen({ raw: true })).toEqual(ansiEscapes.eraseLines(1));
});

it('clear timeout when force closing', { timeout: 1000 }, async () => {
let exitSpy = vi.fn();
const prompt = createPrompt(
(config: { message: string }, done: (value: string) => void) => {
const timeout = useRef<NodeJS.Timeout | undefined>();
const cleaned = useRef(false);
useKeypress(() => {
if (cleaned.current) {
expect.unreachable('once cleaned up keypress should not be called');
}
clearTimeout(timeout.current);
timeout.current = setTimeout(() => {}, 1000);
});

exitSpy = vi.fn(() => {
clearTimeout(timeout.current);
cleaned.current = true;
// We call done explicitly, as onSignalExit is not triggered in this case
// But, CTRL+C will trigger rl.close, which should call this effect
// This way we can have the promise resolve
done('closed');
});

useEffect(() => exitSpy, []);

return config.message;
},
);

const { answer, events } = await render(prompt, { message: 'Question' });

// This triggers the timeout
events.keypress('a');
// This closes the readline
events.keypress({ ctrl: true, name: 'c' });

await expect(answer).resolves.toBe('closed');
expect(exitSpy).toHaveBeenCalledTimes(1);
});
});

it('allow cancelling the prompt multiple times', async () => {
Expand Down
14 changes: 12 additions & 2 deletions packages/core/src/lib/create-prompt.mts
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,16 @@ export function createPrompt<Value, Config>(view: ViewFunction<Value, Config>) {
);
});

const onExit = AsyncResource.bind(() => {
const hooksCleanup = AsyncResource.bind(() => {
try {
effectScheduler.clearAll();
} catch (error) {
reject(error);
}
});

function onExit() {
hooksCleanup();

if (context?.clearPromptOnDone) {
screen.clean();
Expand All @@ -59,7 +63,8 @@ export function createPrompt<Value, Config>(view: ViewFunction<Value, Config>) {

removeExitListener();
rl.input.removeListener('keypress', checkCursorPos);
});
rl.removeListener('close', hooksCleanup);
}

cancel = () => {
onExit();
Expand Down Expand Up @@ -96,6 +101,11 @@ export function createPrompt<Value, Config>(view: ViewFunction<Value, Config>) {
// We set the listener after the initial workLoop to avoid a double render if render triggered
// by a state change sets the cursor to the right position.
rl.input.on('keypress', checkCursorPos);

// The close event triggers immediately when the user press ctrl+c. SignalExit on the other hand
// triggers after the process is done (which happens after timeouts are done triggering.)
// We triggers the hooks cleanup phase on rl `close` so active timeouts can be cleared.
rl.on('close', hooksCleanup);
});
});

Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/lib/use-keypress.mts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ export function useKeypress(
signal.current = userHandler;

useEffect((rl) => {
let ignore = false;
const handler = withUpdates((_input: string, event: KeypressEvent) => {
if (ignore) return;
signal.current(event, rl);
});

rl.input.on('keypress', handler);
return () => {
ignore = true;
rl.input.removeListener('keypress', handler);
};
}, []);
Expand Down
8 changes: 8 additions & 0 deletions packages/select/src/index.mts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
usePagination,
useRef,
useMemo,
useEffect,
isBackspaceKey,
isEnterKey,
isUpKey,
Expand Down Expand Up @@ -149,6 +150,13 @@ export default createPrompt(
}
});

useEffect(
() => () => {
clearTimeout(searchTimeoutRef.current);
},
[],
);

const message = theme.style.message(config.message);

let helpTipTop = '';
Expand Down

0 comments on commit 3610c68

Please sign in to comment.