Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use stdin readable event instead of data #616

Merged
merged 2 commits into from
Sep 1, 2023
Merged

Use stdin readable event instead of data #616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4a564e
Use stdin `readable` event instead of `data`
matteodepalo Aug 4, 2023
9c718b8
Add an internal_eventEmitter to avoid using data event
matteodepalo Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/components/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,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 @@ -471,21 +469,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 @@ -495,8 +487,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 @@ -540,8 +530,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 @@ -551,8 +539,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 @@ -602,20 +588,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
View file
Open in desktop
Original file line number Diff line number Diff line change
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