Skip to content

Commit

Permalink
Use readable event instead of data to read from stdin (#616)
Browse files Browse the repository at this point in the history
  • Loading branch information
matteodepalo committed Sep 1, 2023
1 parent a5b407d commit 814f33e
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 49 deletions.
23 changes: 18 additions & 5 deletions src/components/App.tsx
@@ -1,3 +1,4 @@
import {EventEmitter} from 'node:events';
import process from 'node:process';
import React, {PureComponent, type ReactNode} from 'react';
import cliCursor from 'cli-cursor';
Expand Down Expand Up @@ -55,6 +56,8 @@ export default class App extends PureComponent<Props, State> {
// Count how many components enabled raw mode to avoid disabling
// raw mode until all components don't need it anymore
rawModeEnabledCount = 0;
// eslint-disable-next-line @typescript-eslint/naming-convention
internal_eventEmitter = new EventEmitter();

// Determines if TTY is supported on the provided stdin
isRawModeSupported(): boolean {
Expand All @@ -76,7 +79,9 @@ export default class App extends PureComponent<Props, State> {
setRawMode: this.handleSetRawMode,
isRawModeSupported: this.isRawModeSupported(),
// eslint-disable-next-line @typescript-eslint/naming-convention
internal_exitOnCtrlC: this.props.exitOnCtrlC
internal_exitOnCtrlC: this.props.exitOnCtrlC,
// eslint-disable-next-line @typescript-eslint/naming-convention
internal_eventEmitter: this.internal_eventEmitter
}}
>
<StdoutContext.Provider
Expand Down Expand Up @@ -158,9 +163,8 @@ export default class App extends PureComponent<Props, State> {
if (isEnabled) {
// Ensure raw mode is enabled only once
if (this.rawModeEnabledCount === 0) {
stdin.addListener('data', this.handleInput);
stdin.resume();
stdin.setRawMode(true);
stdin.addListener('readable', this.handleReadable);
}

this.rawModeEnabledCount++;
Expand All @@ -170,8 +174,17 @@ export default class App extends PureComponent<Props, State> {
// Disable raw mode only when no components left that are using it
if (--this.rawModeEnabledCount === 0) {
stdin.setRawMode(false);
stdin.removeListener('data', this.handleInput);
stdin.pause();
stdin.removeListener('readable', this.handleReadable);
stdin.unref();
}
};

handleReadable = (): void => {
let chunk;
// eslint-disable-next-line @typescript-eslint/ban-types
while ((chunk = this.props.stdin.read() as string | null) !== null) {
this.handleInput(chunk);
this.internal_eventEmitter.emit('input', chunk);
}
};

Expand Down
5 changes: 5 additions & 0 deletions src/components/StdinContext.ts
@@ -1,3 +1,4 @@
import {EventEmitter} from 'node:events';
import process from 'node:process';
import {createContext} from 'react';

Expand All @@ -19,6 +20,8 @@ export type Props = {
readonly isRawModeSupported: boolean;

readonly internal_exitOnCtrlC: boolean;

readonly internal_eventEmitter: EventEmitter;
};

/**
Expand All @@ -27,6 +30,8 @@ export type Props = {
// eslint-disable-next-line @typescript-eslint/naming-convention
const StdinContext = createContext<Props>({
stdin: process.stdin,
// eslint-disable-next-line @typescript-eslint/naming-convention
internal_eventEmitter: new EventEmitter(),
setRawMode() {},
isRawModeSupported: false,
// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down
10 changes: 5 additions & 5 deletions src/hooks/use-input.ts
@@ -1,4 +1,3 @@
import {type Buffer} from 'node:buffer';
import {useEffect} from 'react';
import {isUpperCase} from 'is-upper-case';
import parseKeypress, {nonAlphanumericKeys} from '../parse-keypress.js';
Expand Down Expand Up @@ -118,7 +117,8 @@ type Options = {
*/
const useInput = (inputHandler: Handler, options: Options = {}) => {
// eslint-disable-next-line @typescript-eslint/naming-convention
const {stdin, setRawMode, internal_exitOnCtrlC} = useStdin();
const {stdin, setRawMode, internal_exitOnCtrlC, internal_eventEmitter} =
useStdin();

useEffect(() => {
if (options.isActive === false) {
Expand All @@ -137,7 +137,7 @@ const useInput = (inputHandler: Handler, options: Options = {}) => {
return;
}

const handleData = (data: Buffer) => {
const handleData = (data: string) => {
const keypress = parseKeypress(data);

const key = {
Expand Down Expand Up @@ -190,10 +190,10 @@ const useInput = (inputHandler: Handler, options: Options = {}) => {
}
};

stdin?.on('data', handleData);
internal_eventEmitter?.on('input', handleData);

return () => {
stdin?.off('data', handleData);
internal_eventEmitter?.removeListener('input', handleData);
};
}, [options.isActive, stdin, internal_exitOnCtrlC, inputHandler]);
};
Expand Down
20 changes: 0 additions & 20 deletions test/components.tsx
Expand Up @@ -448,8 +448,6 @@ test('disable raw mode when all input components are unmounted', t => {
stdin.setEncoding = () => {};
stdin.setRawMode = spy();
stdin.isTTY = true; // Without this, setRawMode will throw
stdin.resume = spy();
stdin.pause = spy();

const options = {
stdout,
Expand Down Expand Up @@ -496,21 +494,15 @@ test('disable raw mode when all input components are unmounted', t => {

t.true(stdin.setRawMode.calledOnce);
t.deepEqual(stdin.setRawMode.firstCall.args, [true]);
t.true(stdin.resume.calledOnce);
t.false(stdin.pause.called);

rerender(<Test renderFirstInput />);

t.true(stdin.setRawMode.calledOnce);
t.true(stdin.resume.calledOnce);
t.false(stdin.pause.called);

rerender(<Test />);

t.true(stdin.setRawMode.calledTwice);
t.deepEqual(stdin.setRawMode.lastCall.args, [false]);
t.true(stdin.resume.calledOnce);
t.true(stdin.pause.calledOnce);
});

test('setRawMode() should throw if raw mode is not supported', t => {
Expand All @@ -520,8 +512,6 @@ test('setRawMode() should throw if raw mode is not supported', t => {
stdin.setEncoding = () => {};
stdin.setRawMode = spy();
stdin.isTTY = false;
stdin.resume = spy();
stdin.pause = spy();

const didCatchInMount = spy();
const didCatchInUnmount = spy();
Expand Down Expand Up @@ -565,8 +555,6 @@ test('setRawMode() should throw if raw mode is not supported', t => {
t.is(didCatchInMount.callCount, 1);
t.is(didCatchInUnmount.callCount, 1);
t.false(stdin.setRawMode.called);
t.false(stdin.resume.called);
t.false(stdin.pause.called);
});

test('render different component based on whether stdin is a TTY or not', t => {
Expand All @@ -576,8 +564,6 @@ test('render different component based on whether stdin is a TTY or not', t => {
stdin.setEncoding = () => {};
stdin.setRawMode = spy();
stdin.isTTY = false;
stdin.resume = spy();
stdin.pause = spy();

const options = {
stdout,
Expand Down Expand Up @@ -627,20 +613,14 @@ test('render different component based on whether stdin is a TTY or not', t => {
);

t.false(stdin.setRawMode.called);
t.false(stdin.resume.called);
t.false(stdin.pause.called);

rerender(<Test renderFirstInput />);

t.false(stdin.setRawMode.called);
t.false(stdin.resume.called);
t.false(stdin.pause.called);

rerender(<Test />);

t.false(stdin.setRawMode.called);
t.false(stdin.resume.called);
t.false(stdin.pause.called);
});

test('render only last frame when run in CI', async t => {
Expand Down
46 changes: 27 additions & 19 deletions test/focus.tsx
Expand Up @@ -2,7 +2,7 @@ import EventEmitter from 'node:events';
import React, {useEffect} from 'react';
import delay from 'delay';
import test from 'ava';
import {spy} from 'sinon';
import {spy, stub} from 'sinon';
import {render, Box, Text, useFocus, useFocusManager} from '../src/index.js';
import createStdout from './helpers/create-stdout.js';

Expand All @@ -11,12 +11,20 @@ const createStdin = () => {
stdin.isTTY = true;
stdin.setRawMode = spy();
stdin.setEncoding = () => {};
stdin.resume = () => {};
stdin.pause = () => {};
stdin.read = stub();
stdin.unref = () => {};

return stdin;
};

const emitReadable = (stdin: NodeJS.WriteStream, chunk: string) => {
const read = stdin.read as ReturnType<typeof stub>;
read.onCall(0).returns(chunk);
read.onCall(1).returns(null);
stdin.emit('readable');
read.reset();
};

type TestProps = {
showFirst?: boolean;
disableSecond?: boolean;
Expand Down Expand Up @@ -134,7 +142,7 @@ test('unfocus active component on Esc', async t => {
});

await delay(100);
stdin.emit('data', '\u001B');
emitReadable(stdin, '\u001B');
await delay(100);
t.is(
(stdout.write as any).lastCall.args[0],
Expand All @@ -152,7 +160,7 @@ test('switch focus to first component on Tab', async t => {
});

await delay(100);
stdin.emit('data', '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
Expand All @@ -171,8 +179,8 @@ test('switch focus to the next component on Tab', async t => {
});

await delay(100);
stdin.emit('data', '\t');
stdin.emit('data', '\t');
emitReadable(stdin, '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
Expand All @@ -191,16 +199,16 @@ test('switch focus to the first component if currently focused component is the
});

await delay(100);
stdin.emit('data', '\t');
stdin.emit('data', '\t');
emitReadable(stdin, '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
(stdout.write as any).lastCall.args[0],
['First', 'Second', 'Third ✔'].join('\n')
);

stdin.emit('data', '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
Expand All @@ -219,7 +227,7 @@ test('skip disabled component on Tab', async t => {
});

await delay(100);
stdin.emit('data', '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
Expand All @@ -238,15 +246,15 @@ test('switch focus to the previous component on Shift+Tab', async t => {
});

await delay(100);
stdin.emit('data', '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
(stdout.write as any).lastCall.args[0],
['First', 'Second ✔', 'Third'].join('\n')
);

stdin.emit('data', '\u001B[Z');
emitReadable(stdin, '\u001B[Z');
await delay(100);

t.is(
Expand All @@ -265,7 +273,7 @@ test('switch focus to the last component if currently focused component is the f
});

await delay(100);
stdin.emit('data', '\u001B[Z');
emitReadable(stdin, '\u001B[Z');

t.is(
(stdout.write as any).lastCall.args[0],
Expand All @@ -283,8 +291,8 @@ test('skip disabled component on Shift+Tab', async t => {
});

await delay(100);
stdin.emit('data', '\u001B[Z');
stdin.emit('data', '\u001B[Z');
emitReadable(stdin, '\u001B[Z');
emitReadable(stdin, '\u001B[Z');
await delay(100);

t.is(
Expand Down Expand Up @@ -324,7 +332,7 @@ test('focus first component after focused component unregisters', async t => {

t.is((stdout.write as any).lastCall.args[0], ['Second', 'Third'].join('\n'));

stdin.emit('data', '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
Expand All @@ -345,7 +353,7 @@ test('toggle focus management', async t => {
await delay(100);
rerender(<Test autoFocus disabled />);
await delay(100);
stdin.emit('data', '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
Expand All @@ -355,7 +363,7 @@ test('toggle focus management', async t => {

rerender(<Test autoFocus />);
await delay(100);
stdin.emit('data', '\t');
emitReadable(stdin, '\t');
await delay(100);

t.is(
Expand Down

0 comments on commit 814f33e

Please sign in to comment.