Skip to content

Commit

Permalink
chore: code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@mdonnalley
mdonnalley committed Aug 15, 2024
1 parent 076d2fc commit cb8afac
Show file tree
Hide file tree
Showing 5 changed files with 259 additions and 238 deletions.
233 changes: 11 additions & 222 deletions src/components/multi-stage-output.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,77 +1,28 @@
import {ux} from '@oclif/core/ux'
import {capitalCase} from 'change-case'
import {Box, Instance, Text, render} from 'ink'
import {Instance, render} from 'ink'
import {env} from 'node:process'
import React from 'react'

import {icons, spinners} from '../design-elements.js'
import {icons} from '../design-elements.js'
import {StageTracker} from '../stage-tracker.js'
import {readableTime} from '../utils.js'
import {Divider} from './divider.js'
import {SpinnerOrError, SpinnerOrErrorOrChildren} from './spinner.js'
import {Timer} from './timer.js'
import {
FormattedKeyValue,
InfoBlock,
KeyValuePair,
SimpleMessage,
StageInfoBlock,
Stages,
StagesProps,
} from './stages.js'

// Taken from https://github.com/sindresorhus/is-in-ci
const isInCi =
env.CI !== '0' &&
env.CI !== 'false' &&
('CI' in env || 'CONTINUOUS_INTEGRATION' in env || Object.keys(env).some((key) => key.startsWith('CI_')))

type Info<T extends Record<string, unknown>> = {
/**
* key-value: Display a key-value pair with a spinner.
* static-key-value: Display a key-value pair without a spinner.
* message: Display a message.
*/
type: 'dynamic-key-value' | 'static-key-value' | 'message'
/**
* Color of the value.
*/
color?: string
/**
* Get the value to display. Takes the data property on the MultiStageComponent as an argument.
* Useful if you want to apply some logic (like rendering a link) to the data before displaying it.
*
* @param data The data property on the MultiStageComponent.
* @returns {string | undefined}
*/
get: (data?: T) => string | undefined
/**
* Whether the value should be bold.
*/
bold?: boolean
}

type KeyValuePair<T extends Record<string, unknown>> = Info<T> & {
/**
* Label of the key-value pair.
*/
label: string
type: 'dynamic-key-value' | 'static-key-value'
}

type SimpleMessage<T extends Record<string, unknown>> = Info<T> & {
type: 'message'
}

type InfoBlock<T extends Record<string, unknown>> = Array<KeyValuePair<T> | SimpleMessage<T>>
type StageInfoBlock<T extends Record<string, unknown>> = Array<
(KeyValuePair<T> & {stage: string}) | (SimpleMessage<T> & {stage: string})
>

export type FormattedKeyValue = {
readonly color?: string
readonly isBold?: boolean
// eslint-disable-next-line react/no-unused-prop-types
readonly label?: string
// -- what's the difference between `string|undefined` and `string?`
readonly value: string | undefined
// eslint-disable-next-line react/no-unused-prop-types
readonly stage?: string
// eslint-disable-next-line react/no-unused-prop-types
readonly type: 'dynamic-key-value' | 'static-key-value' | 'message'
}

type MultiStageOutputOptions<T extends Record<string, unknown>> = {
/**
* Stages to render.
Expand Down Expand Up @@ -117,168 +68,6 @@ type MultiStageOutputOptions<T extends Record<string, unknown>> = {
readonly jsonEnabled: boolean
}

type StagesProps = {
readonly error?: Error | undefined
readonly postStagesBlock?: FormattedKeyValue[]
readonly preStagesBlock?: FormattedKeyValue[]
readonly stageSpecificBlock?: FormattedKeyValue[]
readonly title: string
readonly hasElapsedTime?: boolean
readonly hasStageTime?: boolean
readonly timerUnit?: 'ms' | 's'
readonly stageTracker: StageTracker
}

function StaticKeyValue({color, isBold, label, value}: FormattedKeyValue): React.ReactNode {
if (!value) return
return (
<Box key={label}>
<Text bold={isBold}>{label}: </Text>
<Text color={color}>{value}</Text>
</Box>
)
}

function SimpleMessage({color, isBold, value}: FormattedKeyValue): React.ReactNode {
if (!value) return
return (
<Text bold={isBold} color={color}>
{value}
</Text>
)
}

function Infos({
error,
keyValuePairs,
stage,
}: {
error?: Error
keyValuePairs: FormattedKeyValue[]
stage?: string
}): React.ReactNode {
return (
keyValuePairs
// If stage is provided, only show info for that stage
// otherwise, show all infos that don't have a specified stage
.filter((kv) => (stage ? kv.stage === stage : !kv.stage))
.map((kv) => {
const key = `${kv.label}-${kv.value}`
if (kv.type === 'message') {
return <SimpleMessage key={key} {...kv} />
}

if (kv.type === 'dynamic-key-value') {
return (
<SpinnerOrErrorOrChildren
key={key}
error={error}
label={`${kv.label}:`}
labelPosition="left"
type={spinners.info}
>
{kv.value && (
<Text bold={kv.isBold} color={kv.color}>
{kv.value}
</Text>
)}
</SpinnerOrErrorOrChildren>
)
}

if (kv.type === 'static-key-value') {
return <StaticKeyValue key={key} {...kv} />
}

return null // what's the difference between returning null and returning undefined?
})
)
}

export function Stages({
// it's more idiomatic for React to put each component in its own file
error,
hasElapsedTime = true,
hasStageTime = true,
postStagesBlock,
preStagesBlock,
stageSpecificBlock,
stageTracker,
timerUnit = 'ms',
title,
}: StagesProps): React.ReactNode {
return (
<Box flexDirection="column" paddingTop={1} paddingBottom={1}>
<Divider title={title} />

{preStagesBlock?.length && (
<Box flexDirection="column" marginLeft={1} paddingTop={1}>
<Infos error={error} keyValuePairs={preStagesBlock} />
</Box>
)}

<Box flexDirection="column" marginLeft={1} paddingTop={1}>
{[...stageTracker.entries()].map(([stage, status]) => (
<Box key={stage} flexDirection="column">
<Box>
{(status === 'current' || status === 'failed') && (
<SpinnerOrError error={error} label={capitalCase(stage)} type={spinners.stage} />
)}

{status === 'skipped' && (
<Text color="dim">
{icons.skipped} {capitalCase(stage)} - Skipped
</Text>
)}

{status === 'completed' && (
<Box>
<Text color="green">{icons.completed}</Text>
<Text>{capitalCase(stage)}</Text>
</Box>
)}

{status === 'pending' && (
<Text color="dim">
{icons.pending} {capitalCase(stage)}
</Text>
)}
{status !== 'pending' && status !== 'skipped' && hasStageTime && (
<Box>
<Text> </Text>
<Timer color="dim" isStopped={status === 'completed'} unit={timerUnit} />
</Box>
)}
</Box>

{stageSpecificBlock &&
stageSpecificBlock.length > 0 &&
status !== 'pending' &&
status !== 'skipped' && ( // stageSpecificBlock && stageSpecificBlock.length > 0 => stageSpecificBlock?.length
<Box flexDirection="column" marginLeft={5}>
<Infos error={error} keyValuePairs={stageSpecificBlock} stage={stage} />
</Box>
)}
</Box>
))}
</Box>

{postStagesBlock && postStagesBlock.length > 0 && (
<Box flexDirection="column" marginLeft={1} paddingTop={1}>
<Infos error={error} keyValuePairs={postStagesBlock} />
</Box>
)}

{hasElapsedTime && (
<Box marginLeft={1} paddingTop={1}>
<Text>Elapsed Time: </Text>
<Timer unit={timerUnit} />
</Box>
)}
</Box>
)
}

class CIMultiStageOutput<T extends Record<string, unknown>> {
private data?: Partial<T>
private readonly hasElapsedTime?: boolean
Expand Down
9 changes: 4 additions & 5 deletions src/components/spinner.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React, {useEffect, useState} from 'react'

import {icons} from '../design-elements.js'

export type UseSpinnerProps = {
type UseSpinnerProps = {
/**
* Type of a spinner.
* See [cli-spinners](https://github.com/sindresorhus/cli-spinners) for available spinners.
Expand All @@ -14,12 +14,11 @@ export type UseSpinnerProps = {
readonly type?: SpinnerName
}

export type UseSpinnerResult = {
type UseSpinnerResult = {
frame: string
}

// there's a lot of what look like unnecessary exports.
export function useSpinner({type = 'dots'}: UseSpinnerProps): UseSpinnerResult {
function useSpinner({type = 'dots'}: UseSpinnerProps): UseSpinnerResult {
const [frame, setFrame] = useState(0)
const spinner = spinners[type]

Expand All @@ -41,7 +40,7 @@ export function useSpinner({type = 'dots'}: UseSpinnerProps): UseSpinnerResult {
}
}

export type SpinnerProps = UseSpinnerProps & {
type SpinnerProps = UseSpinnerProps & {
/**
* Label to show near the spinner.
*/
Expand Down
Loading

0 comments on commit cb8afac

Please sign in to comment.