From 5b91af3d31cd547b25431570c70c0c17085b2b8d Mon Sep 17 00:00:00 2001 From: Shaun Stanworth Date: Wed, 12 Jul 2023 17:42:36 +0100 Subject: [PATCH 1/3] Always show the footer in dev, by keeping the task display running --- packages/app/src/cli/services/dev.ts | 5 +- packages/app/src/cli/services/dev/output.ts | 4 +- .../cli/utilities/app/http-reverse-proxy.ts | 2 +- .../node/ui/components/ConcurrentOutput.tsx | 103 ++++++++++-------- 4 files changed, 62 insertions(+), 52 deletions(-) diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index d4b00452a4a..462950ccf37 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -133,11 +133,11 @@ async function dev(options: DevOptions) { const proxyUrl = usingLocalhost ? `${frontendUrl}:${proxyPort}` : frontendUrl const hmrServerPort = frontendConfig?.configuration.hmr_server ? await getAvailableTCPPort() : undefined - let previewUrl + // By default, preview goes to the direct URL for the app. + let previewUrl = buildAppURLForWeb(storeFqdn, apiKey) let shouldUpdateURLs = false if (frontendConfig || backendConfig) { - previewUrl = buildAppURLForWeb(storeFqdn, apiKey) if (options.update) { const newURLs = generatePartnersURLs( exposedUrl, @@ -203,6 +203,7 @@ async function dev(options: DevOptions) { const draftableExtensions = localApp.allExtensions.filter((ext) => ext.isDraftable(unifiedDeployment)) if (previewableExtensions.length > 0) { + // If any previewable extensions, the preview URL should be the dev console approach previewUrl = `${proxyUrl}/extensions/dev-console` const devExt = await devUIExtensionsTarget({ app: localApp, diff --git a/packages/app/src/cli/services/dev/output.ts b/packages/app/src/cli/services/dev/output.ts index fa0b2db795e..49cdd094ba0 100644 --- a/packages/app/src/cli/services/dev/output.ts +++ b/packages/app/src/cli/services/dev/output.ts @@ -57,7 +57,7 @@ export function outputExtensionsMessages(app: AppInterface) { outputThemeExtensionsMessage(app.allExtensions.filter((ext) => ext.isThemeExtension)) } -export function renderDev(renderConcurrentOptions: RenderConcurrentOptions, previewUrl: string | undefined) { +export function renderDev(renderConcurrentOptions: RenderConcurrentOptions, previewUrl: string) { let options = renderConcurrentOptions if (previewUrl) { @@ -86,7 +86,7 @@ export function renderDev(renderConcurrentOptions: RenderConcurrentOptions, prev }, } } - return renderConcurrent(options) + return renderConcurrent({...options, keepOpenAfterStopping: true}) } function outputThemeExtensionsMessage(extensions: ExtensionInstance[]) { diff --git a/packages/app/src/cli/utilities/app/http-reverse-proxy.ts b/packages/app/src/cli/utilities/app/http-reverse-proxy.ts index dc4d7f75d4a..b3fb15110da 100644 --- a/packages/app/src/cli/utilities/app/http-reverse-proxy.ts +++ b/packages/app/src/cli/utilities/app/http-reverse-proxy.ts @@ -40,7 +40,7 @@ export interface ReverseHTTPProxyTarget { } interface Options { - previewUrl: string | undefined + previewUrl: string portNumber: number proxyTargets: ReverseHTTPProxyTarget[] additionalProcesses: OutputProcess[] diff --git a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx index e8769ca6c8a..1c599dade00 100644 --- a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx +++ b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx @@ -1,12 +1,11 @@ import {OutputProcess} from '../../../../public/node/output.js' -import useAsyncAndUnmount from '../hooks/use-async-and-unmount.js' import {AbortSignal} from '../../../../public/node/abort.js' import {handleCtrlC} from '../../ui.js' import {addOrUpdateConcurrentUIEventOutput} from '../../demo-recorder.js' import {treeKill} from '../../tree-kill.js' import useAbortSignal from '../hooks/use-abort-signal.js' -import React, {FunctionComponent, useState} from 'react' -import {Box, Key, Static, Text, useInput, TextProps, useStdin} from 'ink' +import React, {FunctionComponent, useCallback, useEffect, useState} from 'react' +import {Box, Key, Static, Text, useInput, TextProps, useStdin, useApp} from 'ink' import stripAnsi from 'strip-ansi' import figures from 'figures' import {Writable} from 'stream' @@ -26,6 +25,8 @@ export interface ConcurrentOutputProps { shortcuts: Shortcut[] subTitle?: string } + // If set, the component is not automatically unmounted once the processes have all finished + keepOpenAfterStopping?: boolean } interface Chunk { color: TextProps['color'] @@ -77,48 +78,41 @@ const ConcurrentOutput: FunctionComponent = ({ showTimestamps = true, onInput, footer, + keepOpenAfterStopping, }) => { const [processOutput, setProcessOutput] = useState([]) - const concurrentColors: TextProps['color'][] = ['yellow', 'cyan', 'magenta', 'green', 'blue'] + const {exit: unmountInk} = useApp() const prefixColumnSize = Math.max(...processes.map((process) => process.prefix.length)) const {isRawModeSupported} = useStdin() const [state, setState] = useState(ConcurrentOutputState.Running) - function lineColor(index: number) { - const colorIndex = index < concurrentColors.length ? index : index % concurrentColors.length - return concurrentColors[colorIndex]! - } - - const writableStream = (process: OutputProcess, index: number) => { - return new Writable({ - write(chunk, _encoding, next) { - const lines = stripAnsi(chunk.toString('utf8').replace(/(\n)$/, '')).split(/\n/) - addOrUpdateConcurrentUIEventOutput({prefix: process.prefix, index, output: lines.join('\n')}, {footer}) - - setProcessOutput((previousProcessOutput) => [ - ...previousProcessOutput, - { - color: lineColor(index), - prefix: process.prefix, - lines, - }, - ]) - - next() - }, - }) - } - - const runProcesses = () => { - return Promise.all( - processes.map(async (process, index) => { - const stdout = writableStream(process, index) - const stderr = writableStream(process, index) - - await process.action(stdout, stderr, abortSignal) - }), - ) - } + const writableStream = useCallback( + (process: OutputProcess, index: number) => { + const concurrentColors: TextProps['color'][] = ['yellow', 'cyan', 'magenta', 'green', 'blue'] + function lineColor(index: number) { + const colorIndex = index < concurrentColors.length ? index : index % concurrentColors.length + return concurrentColors[colorIndex]! + } + return new Writable({ + write(chunk, _encoding, next) { + const lines = stripAnsi(chunk.toString('utf8').replace(/(\n)$/, '')).split(/\n/) + addOrUpdateConcurrentUIEventOutput({prefix: process.prefix, index, output: lines.join('\n')}, {footer}) + + setProcessOutput((previousProcessOutput) => [ + ...previousProcessOutput, + { + color: lineColor(index), + prefix: process.prefix, + lines, + }, + ]) + + next() + }, + }) + }, + [footer], + ) const {isAborted} = useAbortSignal(abortSignal) @@ -133,14 +127,29 @@ const ConcurrentOutput: FunctionComponent = ({ {isActive: typeof onInput !== 'undefined' && useShortcuts}, ) - useAsyncAndUnmount(runProcesses, { - onFulfilled: () => { - setState(ConcurrentOutputState.Stopped) - }, - onRejected: () => { - setState(ConcurrentOutputState.Stopped) - }, - }) + useEffect(() => { + const runProcesses = () => { + return Promise.all( + processes.map(async (process, index) => { + const stdout = writableStream(process, index) + const stderr = writableStream(process, index) + + await process.action(stdout, stderr, abortSignal) + }), + ) + } + runProcesses() + .then(() => { + if (!keepOpenAfterStopping) { + setState(ConcurrentOutputState.Stopped) + unmountInk() + } + }) + .catch((error) => { + setState(ConcurrentOutputState.Stopped) + unmountInk(error) + }) + }, [abortSignal, processes, writableStream, unmountInk, keepOpenAfterStopping]) const {lineVertical} = figures From 6793adee4d3cd7721a15e4609126bbccb0302f12 Mon Sep 17 00:00:00 2001 From: Matteo Depalo Date: Thu, 13 Jul 2023 11:23:59 +0100 Subject: [PATCH 2/3] Add test for keepRunningAfterProcessesResolve --- packages/app/src/cli/services/dev/output.ts | 2 +- .../ui/components/ConcurrentOutput.test.tsx | 56 ++++++++++++++++++- .../node/ui/components/ConcurrentOutput.tsx | 31 +++++----- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/packages/app/src/cli/services/dev/output.ts b/packages/app/src/cli/services/dev/output.ts index 49cdd094ba0..58a8131796c 100644 --- a/packages/app/src/cli/services/dev/output.ts +++ b/packages/app/src/cli/services/dev/output.ts @@ -86,7 +86,7 @@ export function renderDev(renderConcurrentOptions: RenderConcurrentOptions, prev }, } } - return renderConcurrent({...options, keepOpenAfterStopping: true}) + return renderConcurrent({...options, keepRunningAfterProcessesResolve: true}) } function outputThemeExtensionsMessage(extensions: ExtensionInstance[]) { diff --git a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.test.tsx b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.test.tsx index 50fb4ed8d87..e816240079e 100644 --- a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.test.tsx @@ -186,8 +186,6 @@ describe('ConcurrentOutput', () => { ) await waitForInputsToBeReady() - expect(onInput).toHaveBeenCalledTimes(0) - renderInstance.stdin.write('a') expect(onInput).toHaveBeenCalledTimes(1) expect(onInput.mock.calls[0]![0]).toBe('a') @@ -338,4 +336,58 @@ describe('ConcurrentOutput', () => { " `) }) + + test('renders the shortcuts and accepts inputs when the processes resolve and keepRunningAfterProcessesResolve is true', async () => { + const onInput = vi.fn() + // Given + const backendProcess = { + prefix: 'backend', + action: async (stdout: Writable, _stderr: Writable, _signal: AbortSignal) => { + stdout.write('first backend message') + stdout.write('second backend message') + stdout.write('third backend message') + }, + } + + // When + const renderInstance = render( + , + ) + + await new Promise((resolve) => setTimeout(resolve, 1000)) + + expect(unstyled(getLastFrameAfterUnmount(renderInstance)!).replace(/\d/g, '0')).toMatchInlineSnapshot(` + "0000-00-00 00:00:00 │ backend │ first backend message + 0000-00-00 00:00:00 │ backend │ second backend message + 0000-00-00 00:00:00 │ backend │ third backend message + + › Press p │ preview in your browser + › Press q │ quit + + Preview URL: https://shopify.com + " + `) + + renderInstance.stdin.write('a') + expect(onInput).toHaveBeenCalledTimes(1) + expect(onInput.mock.calls[0]![0]).toBe('a') + }) }) diff --git a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx index 1c599dade00..8187214c3ca 100644 --- a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx +++ b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx @@ -4,7 +4,7 @@ import {handleCtrlC} from '../../ui.js' import {addOrUpdateConcurrentUIEventOutput} from '../../demo-recorder.js' import {treeKill} from '../../tree-kill.js' import useAbortSignal from '../hooks/use-abort-signal.js' -import React, {FunctionComponent, useCallback, useEffect, useState} from 'react' +import React, {FunctionComponent, useCallback, useEffect, useMemo, useState} from 'react' import {Box, Key, Static, Text, useInput, TextProps, useStdin, useApp} from 'ink' import stripAnsi from 'strip-ansi' import figures from 'figures' @@ -26,7 +26,7 @@ export interface ConcurrentOutputProps { subTitle?: string } // If set, the component is not automatically unmounted once the processes have all finished - keepOpenAfterStopping?: boolean + keepRunningAfterProcessesResolve?: boolean } interface Chunk { color: TextProps['color'] @@ -78,21 +78,24 @@ const ConcurrentOutput: FunctionComponent = ({ showTimestamps = true, onInput, footer, - keepOpenAfterStopping, + keepRunningAfterProcessesResolve, }) => { const [processOutput, setProcessOutput] = useState([]) const {exit: unmountInk} = useApp() const prefixColumnSize = Math.max(...processes.map((process) => process.prefix.length)) const {isRawModeSupported} = useStdin() const [state, setState] = useState(ConcurrentOutputState.Running) + const concurrentColors: TextProps['color'][] = useMemo(() => ['yellow', 'cyan', 'magenta', 'green', 'blue'], []) + const lineColor = useCallback( + (index: number) => { + const colorIndex = index < concurrentColors.length ? index : index % concurrentColors.length + return concurrentColors[colorIndex]! + }, + [concurrentColors], + ) const writableStream = useCallback( (process: OutputProcess, index: number) => { - const concurrentColors: TextProps['color'][] = ['yellow', 'cyan', 'magenta', 'green', 'blue'] - function lineColor(index: number) { - const colorIndex = index < concurrentColors.length ? index : index % concurrentColors.length - return concurrentColors[colorIndex]! - } return new Writable({ write(chunk, _encoding, next) { const lines = stripAnsi(chunk.toString('utf8').replace(/(\n)$/, '')).split(/\n/) @@ -111,11 +114,10 @@ const ConcurrentOutput: FunctionComponent = ({ }, }) }, - [footer], + [footer, lineColor], ) const {isAborted} = useAbortSignal(abortSignal) - const useShortcuts = isRawModeSupported && state === ConcurrentOutputState.Running && !isAborted useInput( @@ -128,7 +130,7 @@ const ConcurrentOutput: FunctionComponent = ({ ) useEffect(() => { - const runProcesses = () => { + ;(() => { return Promise.all( processes.map(async (process, index) => { const stdout = writableStream(process, index) @@ -137,10 +139,9 @@ const ConcurrentOutput: FunctionComponent = ({ await process.action(stdout, stderr, abortSignal) }), ) - } - runProcesses() + })() .then(() => { - if (!keepOpenAfterStopping) { + if (!keepRunningAfterProcessesResolve) { setState(ConcurrentOutputState.Stopped) unmountInk() } @@ -149,7 +150,7 @@ const ConcurrentOutput: FunctionComponent = ({ setState(ConcurrentOutputState.Stopped) unmountInk(error) }) - }, [abortSignal, processes, writableStream, unmountInk, keepOpenAfterStopping]) + }, [abortSignal, processes, writableStream, unmountInk, keepRunningAfterProcessesResolve]) const {lineVertical} = figures From 029b4979546bc59fdd0c7528116bb8c8b3856e01 Mon Sep 17 00:00:00 2001 From: Matteo Depalo Date: Thu, 13 Jul 2023 11:26:00 +0100 Subject: [PATCH 3/3] Add changeset --- .changeset/silent-pigs-wonder.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/silent-pigs-wonder.md diff --git a/.changeset/silent-pigs-wonder.md b/.changeset/silent-pigs-wonder.md new file mode 100644 index 00000000000..bca9db49238 --- /dev/null +++ b/.changeset/silent-pigs-wonder.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-kit': patch +'@shopify/app': patch +--- + +Keep showing the footer when running dev with extension only apps