Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .changeset/quiet-prompt-streams.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli': patch
'@shopify/cli-kit': patch
---

Write interactive prompts to stderr by default to keep stdout clear for command results.
5 changes: 5 additions & 0 deletions .changeset/strict-live-theme-confirmation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/cli': patch
---

Require --allow-live for theme dev on a live theme when prompting is unavailable.
2 changes: 2 additions & 0 deletions docs/cli-kit/ui-kit/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ when you want to display it to the terminal, for example in your exception handl

Prompts interrupt the flow of commands to ask the user for some information. There are two main types of prompts: selects and textual prompts. They all take a `message` property which will be displayed as the title of the prompt to the user and is usually in the form of a question. If you forget to use punctuation at the end of `message` we'll add `?` for you.

Prompts read input from stdin and write to stderr by default, so stdout can contain command results. By default, both stdin and stderr must be terminals for prompting. To use a different output stream, pass it as `renderOptions.stdout`.

For select prompts, if the terminal is not tall enough to render all the options the prompt will resize to fit the terminal window and a message will appear warning the user that only a fraction of the items is being displayed. The user can then cycle through the items, including the hidden ones, by using the arrow keys.

#### `renderSelectPrompt`
Expand Down
8 changes: 4 additions & 4 deletions packages/app/src/cli/commands/app/init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,20 @@ vi.mock('../../services/init/validate.js')
vi.mock('@shopify/cli-kit/node/node-package-manager')

let originalStdinIsTTY: boolean | undefined
let originalStdoutIsTTY: boolean | undefined
let originalStderrIsTTY: boolean | undefined

beforeEach(() => {
originalStdinIsTTY = process.stdin.isTTY
originalStdoutIsTTY = process.stdout.isTTY
originalStderrIsTTY = process.stderr.isTTY
vi.unstubAllEnvs()
Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true})
Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true})
Object.defineProperty(process.stderr, 'isTTY', {value: true, configurable: true, writable: true})
vi.stubEnv('CI', '')
})

afterEach(() => {
Object.defineProperty(process.stdin, 'isTTY', {value: originalStdinIsTTY, configurable: true, writable: true})
Object.defineProperty(process.stdout, 'isTTY', {value: originalStdoutIsTTY, configurable: true, writable: true})
Object.defineProperty(process.stderr, 'isTTY', {value: originalStderrIsTTY, configurable: true, writable: true})
})

describe('Init command', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,43 @@
import {PromptLayout} from './PromptLayout.js'
import {PromptState} from '../../hooks/use-prompt.js'
import {render} from '../../../testing/ui.js'
import {describe, expect, test} from 'vitest'
import {Stdout} from '../../../ui.js'
import {SelectInput} from '../SelectInput.js'
import {describe, expect, test, vi} from 'vitest'
import React from 'react'
import {Box, Text} from 'ink'

describe('PromptLayout', async () => {
test('renders choices when the output stream has no terminal row count', async () => {
const stdout = new Stdout({columns: 80})
Reflect.deleteProperty(stdout, 'rows')
const renderInstance = render(
<PromptLayout
message="Which option?"
state={PromptState.Idle}
input={
<SelectInput
items={[
{label: 'First option', value: 'first'},
{label: 'Second option', value: 'second'},
]}
onSubmit={() => {}}
/>
}
/>,
{stdout},
)

try {
await vi.waitFor(() => expect(stdout.lastFrame()).toContain('Second option'))
stdout.emit('resize')
await vi.waitFor(() => expect(stdout.lastFrame()).toContain('First option'))
} finally {
renderInstance.unmount()
renderInstance.cleanup()
}
})

test("doesn't add unnecessary margins when infoTable is an empty array", async () => {
const items = [
{label: 'first', value: 'first'},
Expand Down
Comment thread
isaacroldan marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {Box, measureElement, Text, useStdout, DOMElement} from 'ink'
import figures from 'figures'
import type {InlineToken, LinkToken, TokenItem} from '../token-item.js'

const DEFAULT_TERMINAL_ROWS = 24

export type Message = TokenItem<Exclude<InlineToken, LinkToken>>

interface PromptLayoutProps {
Expand Down Expand Up @@ -38,7 +40,7 @@ const PromptLayout = ({
const [wrapperHeight, setWrapperHeight] = useState(0)
const [promptAreaHeight, setPromptAreaHeight] = useState(0)
const [inputFixedAreaHeight, setInputFixedAreaHeight] = useState(0)
const currentAvailableLines = stdout.rows - promptAreaHeight - inputFixedAreaHeight
const currentAvailableLines = (stdout.rows ?? DEFAULT_TERMINAL_ROWS) - promptAreaHeight - inputFixedAreaHeight
const [availableLines, setAvailableLines] = useState(currentAvailableLines)

const wrapperRef = useCallback(
Expand Down Expand Up @@ -72,7 +74,7 @@ const PromptLayout = ({

useLayoutEffect(() => {
function onResize() {
const newAvailableLines = stdout.rows - promptAreaHeight - inputFixedAreaHeight
const newAvailableLines = (stdout.rows ?? DEFAULT_TERMINAL_ROWS) - promptAreaHeight - inputFixedAreaHeight
if (newAvailableLines !== availableLines) {
setAvailableLines(newAvailableLines)
}
Expand Down
10 changes: 5 additions & 5 deletions packages/cli-kit/src/public/node/base-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,21 @@ import {Flags} from '@oclif/core'
import {Ajv} from 'ajv'

let originalStdinIsTTY: boolean | undefined
let originalStdoutIsTTY: boolean | undefined
let originalStderrIsTTY: boolean | undefined

beforeEach(() => {
originalStdinIsTTY = process.stdin.isTTY
originalStdoutIsTTY = process.stdout.isTTY
originalStderrIsTTY = process.stderr.isTTY
vi.unstubAllEnvs()
// Default: simulate interactive TTY environment
Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true})
Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true})
Object.defineProperty(process.stderr, 'isTTY', {value: true, configurable: true, writable: true})
vi.stubEnv('CI', '')
})

afterEach(() => {
Object.defineProperty(process.stdin, 'isTTY', {value: originalStdinIsTTY, configurable: true, writable: true})
Object.defineProperty(process.stdout, 'isTTY', {value: originalStdoutIsTTY, configurable: true, writable: true})
Object.defineProperty(process.stderr, 'isTTY', {value: originalStderrIsTTY, configurable: true, writable: true})
mockAndCaptureOutput().clear()
})

Expand Down Expand Up @@ -631,7 +631,7 @@ describe('applying environments', async () => {
runTestInTmpDir('does not throw in TTY mode when a non-TTY required argument is missing', async (tmpDir: string) => {
// Given — simulate interactive terminal
Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true})
Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true})
Object.defineProperty(process.stderr, 'isTTY', {value: true, configurable: true, writable: true})
vi.stubEnv('CI', '')

// When
Expand Down
6 changes: 3 additions & 3 deletions packages/cli-kit/src/public/node/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,15 @@ export function terminalSupportsHyperlinks(): boolean {
}

/**
* Check if the standard input and output streams support prompting.
* Check if standard input and standard error are terminals that support prompting.
*
* @returns True if the standard input and output streams support prompting.
* @returns True if standard input and standard error support prompting.
*/
export function terminalSupportsPrompting(): boolean {
if (isInputDisabled() || isTruthy(process.env.CI)) {
return false
}
return Boolean(process.stdin.isTTY && process.stdout.isTTY)
return Boolean(process.stdin.isTTY && process.stderr.isTTY)
Comment thread
isaacroldan marked this conversation as resolved.
}

/**
Expand Down
47 changes: 46 additions & 1 deletion packages/cli-kit/src/public/node/ui.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,23 @@ import {
renderTasks,
renderWarning,
renderSingleTask,
renderSelectPrompt,
renderAutocompletePrompt,
renderTextPrompt,
renderDangerousConfirmationPrompt,
Task,
} from './ui.js'
import {AbortSignal} from './abort.js'
import {BugError, FatalError, AbortError, FatalErrorType} from './error.js'
import {renderCommandEventAsJson, runWithCommandEvents, runWithCommandEventsForCommand} from './command-events.js'
import {mockAndCaptureOutput, withCapturedStandardStreams} from './testing/output.js'
import {outputResult, TokenizedString, unstyled} from './output.js'
import {Stdin} from '../../private/node/testing/ui.js'
import {Stdin, waitForInputsToBeReady} from '../../private/node/testing/ui.js'
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
import supportsHyperlinks from 'supports-hyperlinks'

import {Writable} from 'stream'
import type {RenderOptions} from 'ink'

vi.mock('supports-hyperlinks')

Expand All @@ -30,6 +35,46 @@ afterEach(() => {
mockAndCaptureOutput().clear()
})

test.each([
{
name: 'select',
renderPrompt: (renderOptions: RenderOptions) =>
renderSelectPrompt({message: 'Which option?', choices: [{label: 'First option', value: 'first'}], renderOptions}),
},
{
name: 'autocomplete',
renderPrompt: (renderOptions: RenderOptions) =>
renderAutocompletePrompt({
message: 'Which option?',
choices: [{label: 'First option', value: 'first'}],
renderOptions,
}),
},
{
name: 'text',
renderPrompt: (renderOptions: RenderOptions) =>
renderTextPrompt({message: 'Which option?', defaultValue: 'first', renderOptions}),
},
{
name: 'dangerous confirmation',
renderPrompt: (renderOptions: RenderOptions) =>
renderDangerousConfirmationPrompt({message: 'Which option?', confirmation: 'first', renderOptions}),
input: '\u001b',
},
])('renders $name prompts to stderr by default', async ({renderPrompt, input}) => {
const stdin = new Stdin()

await withCapturedStandardStreams(async (streams) => {
const result = renderPrompt({stdin: stdin as unknown as NodeJS.ReadStream, debug: true, patchConsole: false})
await waitForInputsToBeReady()
stdin.write(input ?? '\r')
await result

expect(streams.stdout()).toBe('')
expect(streams.stderr()).toContain('Which option?')
})
})

describe('renderInfo', async () => {
test('renders info inside a banner', async () => {
// Given
Expand Down
4 changes: 4 additions & 0 deletions packages/cli-kit/src/public/node/ui.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ export async function renderSelectPrompt<T>(
}}
/>,
{
stdout: process.stderr as unknown as NodeJS.WriteStream,
...renderOptions,
exitOnCtrlC: false,
},
Expand Down Expand Up @@ -444,6 +445,7 @@ export async function renderAutocompletePrompt<T>(
}}
/>,
{
stdout: process.stderr as unknown as NodeJS.WriteStream,
...renderOptions,
exitOnCtrlC: false,
},
Expand Down Expand Up @@ -662,6 +664,7 @@ export async function renderTextPrompt(
}}
/>,
{
stdout: process.stderr as unknown as NodeJS.WriteStream,
...renderOptions,
exitOnCtrlC: false,
},
Expand Down Expand Up @@ -714,6 +717,7 @@ export async function renderDangerousConfirmationPrompt(
}}
/>,
{
stdout: process.stderr as unknown as NodeJS.WriteStream,
...renderOptions,
exitOnCtrlC: false,
},
Expand Down
47 changes: 27 additions & 20 deletions packages/theme/src/cli/utilities/theme-ui.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ vi.mock('@shopify/cli-kit/node/ui')
beforeEach(() => vi.stubEnv('CI', ''))

afterEach(() => {
vi.unstubAllGlobals()
vi.unstubAllEnvs()
})

Expand Down Expand Up @@ -42,7 +43,7 @@ describe('ensureDirectoryConfirmed', () => {
vi.stubGlobal('process', {
...process,
stdin: {...process.stdin, isTTY: true},
stdout: {...process.stdout, isTTY: true},
stderr: {...process.stderr, isTTY: true},
})
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)

Expand All @@ -61,7 +62,7 @@ describe('ensureDirectoryConfirmed', () => {
vi.stubGlobal('process', {
...process,
stdin: {...process.stdin, isTTY: false},
stdout: {...process.stdout, isTTY: true},
stderr: {...process.stderr, isTTY: true},
})

const confirmed = await ensureDirectoryConfirmed(false)
Expand Down Expand Up @@ -103,7 +104,7 @@ describe('ensureLiveThemeConfirmed', () => {
vi.stubGlobal('process', {
...process,
stdin: {...process.stdin, isTTY: true},
stdout: {...process.stdout, isTTY: true},
stderr: {...process.stderr, isTTY: true},
})
})

Expand Down Expand Up @@ -131,35 +132,41 @@ describe('ensureLiveThemeConfirmed', () => {
expect(renderConfirmationPrompt).not.toHaveBeenCalled()
})

test('does not prompt for confirmation if acting on a live theme and allowLive flag is true', async () => {
// Given
await ensureLiveThemeConfirmed(liveTheme, 'start development mode', true)

// Then
expect(renderConfirmationPrompt).not.toHaveBeenCalled()
})

test('requires --allow-live when input is disabled', async () => {
test('allows a live theme with --allow-live when prompting is unavailable', async () => {
vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', 'true')
vi.stubGlobal('process', {
...process,
stdin: {...process.stdin, isTTY: false},
stderr: {...process.stderr, isTTY: false},
})

const confirmation = ensureLiveThemeConfirmed(liveTheme, 'start development mode', false)
const result = await ensureLiveThemeConfirmed(liveTheme, 'start development mode', true)

await expect(confirmation).rejects.toThrow(
"Can't start development mode on the live theme when user input is unavailable.",
)
expect(result).toBe(true)
expect(renderConfirmationPrompt).not.toHaveBeenCalled()
})

test('preserves existing behavior for a live theme in a non-interactive environment', async () => {
test.each([
{name: 'input is disabled', stdin: true, stderr: true, noInput: 'true'},
{name: 'stdin is redirected', stdin: false, stderr: true},
{name: 'stderr is redirected', stdin: true, stderr: false},
{name: 'CI is enabled', stdin: true, stderr: true, ci: 'true'},
])('requires --allow-live when $name', async ({stdin, stderr, noInput, ci}) => {
vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', noInput ?? '')
vi.stubEnv('CI', ci ?? '')
vi.stubGlobal('process', {
...process,
stdin: {...process.stdin, isTTY: false},
stdin: {...process.stdin, isTTY: stdin},
stdout: {...process.stdout, isTTY: true},
stderr: {...process.stderr, isTTY: stderr},
})

const result = await ensureLiveThemeConfirmed(liveTheme, 'start development mode', false)
const confirmation = ensureLiveThemeConfirmed(liveTheme, 'start development mode', false)

expect(result).toBe(true)
await expect(confirmation).rejects.toMatchObject({
message: "Can't start development mode on the live theme when user input is unavailable.",
tryMessage: 'Use `--allow-live` to confirm that you want to continue.',
})
expect(renderConfirmationPrompt).not.toHaveBeenCalled()
})
})
Expand Down
4 changes: 1 addition & 3 deletions packages/theme/src/cli/utilities/theme-ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,13 @@ export async function ensureLiveThemeConfirmed(theme: Theme, action: string, all
return true
}

if (isInputDisabled()) {
if (!terminalSupportsPrompting()) {
throw new AbortError(
`Can't ${action} on the live theme when user input is unavailable.`,
'Use `--allow-live` to confirm that you want to continue.',
)
}

if (!terminalSupportsPrompting()) return true

const message =
`You're about to ${action} on your live theme "${theme.name}". ` +
`This will make changes visible to customers. Are you sure you want to proceed?`
Expand Down
Loading