From 82406903f30cfed6e899bb327d341a03f05ff67e Mon Sep 17 00:00:00 2001 From: Tom Sherman Date: Thu, 11 Apr 2024 12:17:07 +0100 Subject: [PATCH 1/5] Refactor app to use function components and hooks --- src/components/App.tsx | 511 ++++++++++++++++++++--------------------- 1 file changed, 250 insertions(+), 261 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 46247945..2f5cfc59 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -1,6 +1,14 @@ import {EventEmitter} from 'node:events'; import process from 'node:process'; -import React, {PureComponent, type ReactNode} from 'react'; +import React, { + Component, + PureComponent, + useCallback, + useEffect, + useRef, + useState, + type ReactNode +} from 'react'; import cliCursor from 'cli-cursor'; import AppContext from './AppContext.js'; import StdinContext from './StdinContext.js'; @@ -39,121 +47,54 @@ type Focusable = { // Root component for all Ink apps // It renders stdin and stdout contexts, so that children can access them if needed // It also handles Ctrl+C exiting and cursor visibility -export default class App extends PureComponent { - static displayName = 'InternalApp'; - - static getDerivedStateFromError(error: Error) { - return {error}; - } - - override state = { - isFocusEnabled: true, - activeFocusId: undefined, - focusables: [], - error: undefined, - }; - - // 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 { - return this.props.stdin.isTTY; - } - - override render() { - return ( - - - - - - {this.state.error ? ( - - ) : ( - this.props.children - )} - - - - - - ); - } - - override componentDidMount() { - cliCursor.hide(this.props.stdout); - } - - override componentWillUnmount() { - cliCursor.show(this.props.stdout); +export default function App({ + stdin, + onExit, + children, + exitOnCtrlC, + stderr, + stdout, + writeToStderr, + writeToStdout +}: Props) { + const rawModeEnabledCountRef = useRef(0); + const internal_eventEmitterRef = useRef(new EventEmitter()); + const [activeFocusId, setActiveFocusId] = useState(); + const [isFocusEnabled, setIsFocusEnabled] = useState(true); + const [focusables, setFocusables] = useState([]); + + const isRawModeSupported = stdin.isTTY; + + useEffect(() => { + cliCursor.hide(stdout); + + return () => { + cliCursor.show(stdout); + + // ignore calling setRawMode on an handle stdin it cannot be called + if (isRawModeSupported) { + handleSetRawMode(false); + } + }; + }, [stdout, isRawModeSupported]); - // ignore calling setRawMode on an handle stdin it cannot be called - if (this.isRawModeSupported()) { - this.handleSetRawMode(false); + function handleExit(error?: Error) { + if (isRawModeSupported) { + handleSetRawMode(false); } - } - override componentDidCatch(error: Error) { - this.handleExit(error); + onExit(error); } - handleSetRawMode = (isEnabled: boolean): void => { - const {stdin} = this.props; - - if (!this.isRawModeSupported()) { + function handleSetRawMode(isEnabled?: boolean) { + if (!isRawModeSupported) { if (stdin === process.stdin) { throw new Error( - 'Raw mode is not supported on the current process.stdin, which Ink uses as input stream by default.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported', + 'Raw mode is not supported on the current process.stdin, which Ink uses as input stream by default.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported' ); } else { throw new Error( - 'Raw mode is not supported on the stdin provided to Ink.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported', + 'Raw mode is not supported on the stdin provided to Ink.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported' ); } } @@ -162,215 +103,263 @@ export default class App extends PureComponent { if (isEnabled) { // Ensure raw mode is enabled only once - if (this.rawModeEnabledCount === 0) { + if (rawModeEnabledCountRef.current === 0) { stdin.ref(); stdin.setRawMode(true); - stdin.addListener('readable', this.handleReadable); + stdin.addListener('readable', handleReadable); } - this.rawModeEnabledCount++; + rawModeEnabledCountRef.current++; return; } // Disable raw mode only when no components left that are using it - if (--this.rawModeEnabledCount === 0) { + if (--rawModeEnabledCountRef.current === 0) { stdin.setRawMode(false); - stdin.removeListener('readable', this.handleReadable); + stdin.removeListener('readable', handleReadable); stdin.unref(); } - }; + } - handleReadable = (): void => { + // TODO: Do the setStates in this function need to be wrapped in flushSync? + function handleReadable() { 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); - } - }; - - handleInput = (input: string): void => { - // Exit on Ctrl+C - // eslint-disable-next-line unicorn/no-hex-escape - if (input === '\x03' && this.props.exitOnCtrlC) { - this.handleExit(); - } - - // Reset focus when there's an active focused component on Esc - if (input === escape && this.state.activeFocusId) { - this.setState({ - activeFocusId: undefined, - }); - } - - if (this.state.isFocusEnabled && this.state.focusables.length > 0) { - if (input === tab) { - this.focusNext(); + while ((chunk = stdin.read() as string | null) !== null) { + // Exit on Ctrl+C + // eslint-disable-next-line unicorn/no-hex-escape + if (chunk === '\x03' && exitOnCtrlC) { + handleExit(); } - if (input === shiftTab) { - this.focusPrevious(); + // Reset focus when there's an active focused component on Esc + if (chunk === escape && activeFocusId) { + setActiveFocusId(undefined); } - } - }; - handleExit = (error?: Error): void => { - if (this.isRawModeSupported()) { - this.handleSetRawMode(false); - } - - this.props.onExit(error); - }; - - enableFocus = (): void => { - this.setState({ - isFocusEnabled: true, - }); - }; - - disableFocus = (): void => { - this.setState({ - isFocusEnabled: false, - }); - }; - - focus = (id: string): void => { - this.setState(previousState => { - const hasFocusableId = previousState.focusables.some( - focusable => focusable?.id === id, - ); + if (isFocusEnabled && focusables.length > 0) { + if (chunk === tab) { + focusNext(); + } - if (!hasFocusableId) { - return previousState; + if (chunk === shiftTab) { + focusPrevious(); + } } + internal_eventEmitterRef.current.emit('input', chunk); + } + } - return {activeFocusId: id}; - }); - }; - - focusNext = (): void => { - this.setState(previousState => { - const firstFocusableId = previousState.focusables[0]?.id; - const nextFocusableId = this.findNextFocusable(previousState); - - return { - activeFocusId: nextFocusableId ?? firstFocusableId, - }; - }); - }; + function focus(id: string): void { + const hasFocusableId = focusables.some(focusable => focusable?.id === id); - focusPrevious = (): void => { - this.setState(previousState => { - const lastFocusableId = previousState.focusables.at(-1)?.id; + if (!hasFocusableId) { + return; + } - const previousFocusableId = this.findPreviousFocusable(previousState); + setActiveFocusId(id); + } - return { - activeFocusId: previousFocusableId ?? lastFocusableId, - }; - }); - }; + function focusNext() { + const firstFocusableId = focusables[0]?.id; + const nextFocusableId = findNextFocusable(focusables, activeFocusId); + setActiveFocusId(nextFocusableId ?? firstFocusableId); + } - addFocusable = (id: string, {autoFocus}: {autoFocus: boolean}): void => { - this.setState(previousState => { - let nextFocusId = previousState.activeFocusId; + function focusPrevious() { + const lastFocusableId = focusables.at(-1)?.id; + const previousFocusableId = findPreviousFocusable( + focusables, + activeFocusId + ); + setActiveFocusId(previousFocusableId ?? lastFocusableId); + } - if (!nextFocusId && autoFocus) { - nextFocusId = id; + function addFocusable(id: string, {autoFocus}: {autoFocus: boolean}): void { + let nextFocusId = activeFocusId; + if (!nextFocusId && autoFocus) { + nextFocusId = id; + } + setActiveFocusId(nextFocusId); + setFocusables([ + ...focusables, + { + id, + isActive: true } + ]); + } - return { - activeFocusId: nextFocusId, - focusables: [ - ...previousState.focusables, - { - id, - isActive: true, - }, - ], - }; - }); - }; - - removeFocusable = (id: string): void => { - this.setState(previousState => ({ - activeFocusId: - previousState.activeFocusId === id - ? undefined - : previousState.activeFocusId, - focusables: previousState.focusables.filter(focusable => { + function removeFocusable(id: string): void { + setActiveFocusId(activeFocusId === id ? undefined : activeFocusId); + setFocusables( + focusables.filter(focusable => { return focusable.id !== id; - }), - })); - }; + }) + ); + } - activateFocusable = (id: string): void => { - this.setState(previousState => ({ - focusables: previousState.focusables.map(focusable => { + function activateFocusable(id: string): void { + setFocusables( + focusables.map(focusable => { if (focusable.id !== id) { return focusable; } return { id, - isActive: true, + isActive: true }; - }), - })); - }; + }) + ); + } - deactivateFocusable = (id: string): void => { - this.setState(previousState => ({ - activeFocusId: - previousState.activeFocusId === id - ? undefined - : previousState.activeFocusId, - focusables: previousState.focusables.map(focusable => { + function deactivateFocusable(id: string): void { + setActiveFocusId(activeFocusId === id ? undefined : activeFocusId); + setFocusables( + focusables.map(focusable => { if (focusable.id !== id) { return focusable; } return { id, - isActive: false, + isActive: false }; - }), - })); - }; + }) + ); + } - findNextFocusable = (state: State): string | undefined => { - const activeIndex = state.focusables.findIndex(focusable => { - return focusable.id === state.activeFocusId; - }); + function enableFocus(): void { + setIsFocusEnabled(true); + } - for ( - let index = activeIndex + 1; - index < state.focusables.length; - index++ - ) { - const focusable = state.focusables[index]; + function disableFocus(): void { + setIsFocusEnabled(false); + } - if (focusable?.isActive) { - return focusable.id; - } + return ( + + + + + + + {children} + + + + + + + ); +} +App.displayName = 'InternalApp'; + +function findPreviousFocusable( + focusables: Focusable[], + activeFocusId: string | undefined +): string | undefined { + const activeIndex = focusables.findIndex(focusable => { + return focusable.id === activeFocusId; + }); + + for (let index = activeIndex - 1; index >= 0; index--) { + const focusable = focusables[index]; + + if (focusable?.isActive) { + return focusable.id; } + } - return undefined; - }; + return undefined; +} - findPreviousFocusable = (state: State): string | undefined => { - const activeIndex = state.focusables.findIndex(focusable => { - return focusable.id === state.activeFocusId; - }); +function findNextFocusable( + focusables: Focusable[], + activeFocusId: string | undefined +): string | undefined { + const activeIndex = focusables.findIndex(focusable => { + return focusable.id === activeFocusId; + }); - for (let index = activeIndex - 1; index >= 0; index--) { - const focusable = state.focusables[index]; + for (let index = activeIndex + 1; index < focusables.length; index++) { + const focusable = focusables[index]; - if (focusable?.isActive) { - return focusable.id; - } + if (focusable?.isActive) { + return focusable.id; } + } - return undefined; + return undefined; +} + +type ErrorState = { + error?: Error; +}; + +class InternalAppErrorBoundary extends Component< + {handleExit: (error: Error) => void; children: ReactNode}, + ErrorState +> { + state = { + error: undefined }; + + static getDerivedStateFromError(error: Error) { + return {error}; + } + + override componentDidCatch(error: Error) { + this.props.handleExit(error); + } + + override render() { + return this.state.error ? ( + + ) : ( + this.props.children + ); + } } From cad606afda8c01f835754f00350e72d49693525c Mon Sep 17 00:00:00 2001 From: Tom Sherman Date: Thu, 11 Apr 2024 14:32:25 +0100 Subject: [PATCH 2/5] Remove unapplicable todo comment --- src/components/App.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 2f5cfc59..4651f8c6 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -121,7 +121,6 @@ export default function App({ } } - // TODO: Do the setStates in this function need to be wrapped in flushSync? function handleReadable() { let chunk; // eslint-disable-next-line @typescript-eslint/ban-types From 5c764c260a71222bffe2e3fe9369d0572be5f672 Mon Sep 17 00:00:00 2001 From: Tom Sherman Date: Thu, 11 Apr 2024 14:51:02 +0100 Subject: [PATCH 3/5] Rearrange closures to match existing order of methods for better diff --- src/components/App.tsx | 92 +++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 4651f8c6..f08a8cee 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -2,8 +2,6 @@ import {EventEmitter} from 'node:events'; import process from 'node:process'; import React, { Component, - PureComponent, - useCallback, useEffect, useRef, useState, @@ -44,6 +42,35 @@ type Focusable = { readonly isActive: boolean; }; +type ErrorState = { + error?: Error; +}; + +class InternalAppErrorBoundary extends Component< + {handleExit: (error: Error) => void; children: ReactNode}, + ErrorState +> { + static getDerivedStateFromError(error: Error) { + return {error}; + } + + override state = { + error: undefined + }; + + override componentDidCatch(error: Error) { + this.props.handleExit(error); + } + + override render() { + return this.state.error ? ( + + ) : ( + this.props.children + ); + } +} + // Root component for all Ink apps // It renders stdin and stdout contexts, so that children can access them if needed // It also handles Ctrl+C exiting and cursor visibility @@ -78,14 +105,6 @@ export default function App({ }; }, [stdout, isRawModeSupported]); - function handleExit(error?: Error) { - if (isRawModeSupported) { - handleSetRawMode(false); - } - - onExit(error); - } - function handleSetRawMode(isEnabled?: boolean) { if (!isRawModeSupported) { if (stdin === process.stdin) { @@ -149,6 +168,22 @@ export default function App({ } } + function handleExit(error?: Error) { + if (isRawModeSupported) { + handleSetRawMode(false); + } + + onExit(error); + } + + function enableFocus(): void { + setIsFocusEnabled(true); + } + + function disableFocus(): void { + setIsFocusEnabled(false); + } + function focus(id: string): void { const hasFocusableId = focusables.some(focusable => focusable?.id === id); @@ -229,14 +264,6 @@ export default function App({ ); } - function enableFocus(): void { - setIsFocusEnabled(true); - } - - function disableFocus(): void { - setIsFocusEnabled(false); - } - return ( void; children: ReactNode}, - ErrorState -> { - state = { - error: undefined - }; - - static getDerivedStateFromError(error: Error) { - return {error}; - } - - override componentDidCatch(error: Error) { - this.props.handleExit(error); - } - - override render() { - return this.state.error ? ( - - ) : ( - this.props.children - ); - } -} From 2a494e9d5afa56cd29e408cece401a4289b756ef Mon Sep 17 00:00:00 2001 From: Tom Sherman Date: Thu, 11 Apr 2024 14:51:13 +0100 Subject: [PATCH 4/5] Remove unused state type --- src/components/App.tsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index f08a8cee..1737d852 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -30,13 +30,6 @@ type Props = { readonly onExit: (error?: Error) => void; }; -type State = { - readonly isFocusEnabled: boolean; - readonly activeFocusId?: string; - readonly focusables: Focusable[]; - readonly error?: Error; -}; - type Focusable = { readonly id: string; readonly isActive: boolean; From 39a08199458208c3b9da4b0f56d2b4e043825ff0 Mon Sep 17 00:00:00 2001 From: Tom Sherman Date: Thu, 11 Apr 2024 15:31:53 +0100 Subject: [PATCH 5/5] Linting --- src/components/App.tsx | 65 +++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 1737d852..72424988 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -5,7 +5,7 @@ import React, { useEffect, useRef, useState, - type ReactNode + type ReactNode, } from 'react'; import cliCursor from 'cli-cursor'; import AppContext from './AppContext.js'; @@ -48,7 +48,7 @@ class InternalAppErrorBoundary extends Component< } override state = { - error: undefined + error: undefined, }; override componentDidCatch(error: Error) { @@ -75,10 +75,10 @@ export default function App({ stderr, stdout, writeToStderr, - writeToStdout + writeToStdout, }: Props) { const rawModeEnabledCountRef = useRef(0); - const internal_eventEmitterRef = useRef(new EventEmitter()); + const eventEmitterRef = useRef(new EventEmitter()); const [activeFocusId, setActiveFocusId] = useState(); const [isFocusEnabled, setIsFocusEnabled] = useState(true); const [focusables, setFocusables] = useState([]); @@ -102,11 +102,11 @@ export default function App({ if (!isRawModeSupported) { if (stdin === process.stdin) { throw new Error( - 'Raw mode is not supported on the current process.stdin, which Ink uses as input stream by default.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported' + 'Raw mode is not supported on the current process.stdin, which Ink uses as input stream by default.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported', ); } else { throw new Error( - 'Raw mode is not supported on the stdin provided to Ink.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported' + 'Raw mode is not supported on the stdin provided to Ink.\nRead about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported', ); } } @@ -157,7 +157,8 @@ export default function App({ focusPrevious(); } } - internal_eventEmitterRef.current.emit('input', chunk); + + eventEmitterRef.current.emit('input', chunk); } } @@ -197,7 +198,7 @@ export default function App({ const lastFocusableId = focusables.at(-1)?.id; const previousFocusableId = findPreviousFocusable( focusables, - activeFocusId + activeFocusId, ); setActiveFocusId(previousFocusableId ?? lastFocusableId); } @@ -207,13 +208,14 @@ export default function App({ if (!nextFocusId && autoFocus) { nextFocusId = id; } + setActiveFocusId(nextFocusId); setFocusables([ ...focusables, { id, - isActive: true - } + isActive: true, + }, ]); } @@ -222,7 +224,7 @@ export default function App({ setFocusables( focusables.filter(focusable => { return focusable.id !== id; - }) + }), ); } @@ -235,9 +237,9 @@ export default function App({ return { id, - isActive: true + isActive: true, }; - }) + }), ); } @@ -251,42 +253,43 @@ export default function App({ return { id, - isActive: false + isActive: false, }; - }) + }), ); } return ( + {/* eslint-disable-next-line react/jsx-no-bind */} {children} @@ -314,11 +318,12 @@ export default function App({ ); } + App.displayName = 'InternalApp'; function findPreviousFocusable( focusables: Focusable[], - activeFocusId: string | undefined + activeFocusId: string | undefined, ): string | undefined { const activeIndex = focusables.findIndex(focusable => { return focusable.id === activeFocusId; @@ -337,7 +342,7 @@ function findPreviousFocusable( function findNextFocusable( focusables: Focusable[], - activeFocusId: string | undefined + activeFocusId: string | undefined, ): string | undefined { const activeIndex = focusables.findIndex(focusable => { return focusable.id === activeFocusId;