Merge pull request #6361 from Shopify/jb-multi-env-push-share

Allow theme push and share commands to be called with multiple environments

Author: dejmedus

.changeset/clever-ducks-sleep.md

@@ -0,0 +1,7 @@
+---
+'@shopify/theme': minor
+'@shopify/cli': minor
+'@shopify/cli-kit': minor
+---
+
+Allow theme push and share commands to be called with multiple environments

.changeset/light-ears-drum.md

@@ -0,0 +1,6 @@
+---
+'@shopify/theme': patch
+'@shopify/cli': patch
+---
+
+Remove leftover references to CLI2 from theme commands

packages/cli-kit/src/private/node/ui/components/LoadingBar.test.tsx

@@ -204,4 +204,16 @@ describe('LoadingBar', () => {
     // Then
     expect(frame1()).toBe(frame2())
   })
+
+  test('hides progress bar when noProgressBar is true', async () => {
+    // Given
+    vi.mocked(shouldDisplayColors).mockReturnValue(true)
+    const title = 'task 1'
+
+    // When
+    const {lastFrame} = render(<LoadingBar title={title} noProgressBar />)
+
+    // Then
+    expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`"task 1 ..."`)
+  })
 })

packages/cli-kit/src/private/node/ui/components/LoadingBar.tsx

@@ -10,9 +10,10 @@ const hillString = '▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆
 interface LoadingBarProps {
   title: string
   noColor?: boolean
+  noProgressBar?: boolean
 }
 
-const LoadingBar = ({title, noColor}: React.PropsWithChildren<LoadingBarProps>) => {
+const LoadingBar = ({title, noColor, noProgressBar}: React.PropsWithChildren<LoadingBarProps>) => {
   const {twoThirds} = useLayout()
   let loadingBar = new Array(twoThirds).fill(loadingBarChar).join('')
   if (noColor ?? !shouldDisplayColors()) {
@@ -21,7 +22,7 @@ const LoadingBar = ({title, noColor}: React.PropsWithChildren<LoadingBarProps>)
 
   return (
     <Box flexDirection="column">
-      <TextAnimation text={loadingBar} maxWidth={twoThirds} />
+      {!noProgressBar && <TextAnimation text={loadingBar} maxWidth={twoThirds} />}
       <Text>{title} ...</Text>
     </Box>
   )

packages/cli-kit/src/private/node/ui/components/Tasks.tsx

@@ -22,6 +22,7 @@ interface TasksProps<TContext> {
   onComplete?: (ctx: TContext) => void
   abortSignal?: AbortSignal
   noColor?: boolean
+  noProgressBar?: boolean
 }
 
 enum TasksState {
@@ -63,6 +64,7 @@ function Tasks<TContext>({
   onComplete = noop,
   abortSignal,
   noColor,
+  noProgressBar = false,
 }: React.PropsWithChildren<TasksProps<TContext>>) {
   // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
   const [currentTask, setCurrentTask] = useState<Task<TContext>>(tasks[0]!)
@@ -105,7 +107,9 @@ function Tasks<TContext>({
     return null
   }
 
-  return state === TasksState.Loading && !isAborted ? <LoadingBar title={currentTask.title} noColor={noColor} /> : null
+  return state === TasksState.Loading && !isAborted ? (
+    <LoadingBar title={currentTask.title} noColor={noColor} noProgressBar={noProgressBar} />
+  ) : null
 }
 
 export {Tasks}

packages/cli-kit/src/public/node/ui.tsx

@@ -462,6 +462,7 @@ export function renderTable<T extends ScalarDict>({renderOptions, ...props}: Ren
 
 interface RenderTasksOptions {
   renderOptions?: RenderOptions
+  noProgressBar?: boolean
 }
 
 /**
@@ -471,10 +472,13 @@ interface RenderTasksOptions {
  * Installing dependencies ...
  */
 // eslint-disable-next-line max-params
-export async function renderTasks<TContext>(tasks: Task<TContext>[], {renderOptions}: RenderTasksOptions = {}) {
+export async function renderTasks<TContext>(
+  tasks: Task<TContext>[],
+  {renderOptions, noProgressBar}: RenderTasksOptions = {},
+) {
   // eslint-disable-next-line max-params
   return new Promise<TContext>((resolve, reject) => {
-    render(<Tasks tasks={tasks} onComplete={resolve} />, {
+    render(<Tasks tasks={tasks} onComplete={resolve} noProgressBar={noProgressBar} />, {
       ...renderOptions,
       exitOnCtrlC: false,
     })

packages/cli/oclif.manifest.json

@@ -6674,6 +6674,16 @@
       "hiddenAliases": [
       ],
       "id": "theme:push",
+      "multiEnvironmentsFlags": [
+        "store",
+        "password",
+        "path",
+        [
+          "live",
+          "development",
+          "theme"
+        ]
+      ],
       "pluginAlias": "@shopify/cli",
       "pluginName": "@shopify/cli",
       "pluginType": "core",
@@ -7001,9 +7011,6 @@
       ],
       "args": {
       },
-      "cli2Flags": [
-        "force"
-      ],
       "customPluginName": "@shopify/theme",
       "description": "Uploads your theme as a new, unpublished theme in your theme library. The theme is given a randomized name.\n\n  This command returns a \"preview link\" (https://help.shopify.com/manual/online-store/themes/adding-themes#share-a-theme-preview-with-others) that you can share with others.",
       "descriptionWithMarkdown": "Uploads your theme as a new, unpublished theme in your theme library. The theme is given a randomized name.\n\n  This command returns a [preview link](https://help.shopify.com/manual/online-store/themes/adding-themes#share-a-theme-preview-with-others) that you can share with others.",
@@ -7073,6 +7080,11 @@
       "hiddenAliases": [
       ],
       "id": "theme:share",
+      "multiEnvironmentsFlags": [
+        "store",
+        "password",
+        "path"
+      ],
       "pluginAlias": "@shopify/cli",
       "pluginName": "@shopify/cli",
       "pluginType": "core",

packages/theme/src/cli/commands/theme/push.ts

@@ -1,9 +1,14 @@
 import {globFlags, themeFlags} from '../../flags.js'
 import ThemeCommand from '../../utilities/theme-command.js'
-import {push, PushFlags} from '../../services/push.js'
+import {push} from '../../services/push.js'
 import {Flags} from '@oclif/core'
 import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
 import {recordTiming} from '@shopify/cli-kit/node/analytics'
+import {AdminSession} from '@shopify/cli-kit/node/session'
+import {InferredFlags} from '@oclif/core/interfaces'
+import {Writable} from 'stream'
+
+type PushFlags = InferredFlags<typeof Push.flags>
 
 export default class Push extends ThemeCommand {
   static summary = 'Uploads your local theme files to the connected store, overwriting the remote version if specified.'
@@ -91,33 +96,28 @@ export default class Push extends ThemeCommand {
       description: 'Require theme check to pass without errors before pushing. Warnings are allowed.',
       env: 'SHOPIFY_FLAG_STRICT_PUSH',
     }),
+    environment: themeFlags.environment,
   }
 
-  async run(): Promise<void> {
-    const {flags} = await this.parse(Push)
-
-    const pushFlags: PushFlags = {
-      path: flags.path,
-      password: flags.password,
-      store: flags.store,
-      theme: flags.theme,
-      development: flags.development,
-      live: flags.live,
-      unpublished: flags.unpublished,
-      nodelete: flags.nodelete,
-      only: flags.only,
-      ignore: flags.ignore,
-      json: flags.json,
-      allowLive: flags['allow-live'],
-      publish: flags.publish,
-      force: flags.force,
-      noColor: flags['no-color'],
-      verbose: flags.verbose,
-      strict: flags.strict,
-    }
+  static multiEnvironmentsFlags = ['store', 'password', 'path', ['live', 'development', 'theme']]
 
+  async command(
+    flags: PushFlags,
+    adminSession: AdminSession,
+    multiEnvironment: boolean,
+    context?: {stdout?: Writable; stderr?: Writable},
+  ) {
     recordTiming('theme-command:push')
-    await push(pushFlags)
+    await push(
+      {
+        ...flags,
+        allowLive: flags['allow-live'],
+        noColor: flags['no-color'],
+      },
+      adminSession,
+      multiEnvironment,
+      context,
+    )
     recordTiming('theme-command:push')
   }
 }

packages/theme/src/cli/commands/theme/share.ts

@@ -5,7 +5,11 @@ import {Flags} from '@oclif/core'
 import {globalFlags} from '@shopify/cli-kit/node/cli'
 import {getRandomName} from '@shopify/cli-kit/common/string'
 import {recordTiming} from '@shopify/cli-kit/node/analytics'
+import {InferredFlags} from '@oclif/core/interfaces'
+import {AdminSession} from '@shopify/cli-kit/node/session'
+import {Writable} from 'stream'
 
+type ShareFlags = InferredFlags<typeof Share.flags>
 export default class Share extends ThemeCommand {
   static summary = 'Creates a shareable, unpublished, and new theme on your theme library with a randomized name.'
 
@@ -26,22 +30,27 @@ export default class Share extends ThemeCommand {
     }),
   }
 
-  static cli2Flags = ['force']
-
-  async run(): Promise<void> {
-    const {flags} = await this.parse(Share)
+  static multiEnvironmentsFlags = ['store', 'password', 'path']
 
+  async command(
+    flags: ShareFlags,
+    adminSession: AdminSession,
+    multiEnvironment: boolean,
+    context?: {stdout?: Writable; stderr?: Writable},
+  ) {
     const pushFlags: PushFlags = {
+      environment: flags.environment,
       force: flags.force,
-      path: flags.path,
+      noColor: flags['no-color'],
       password: flags.password,
+      path: flags.path,
       store: flags.store,
-      unpublished: true,
       theme: getRandomName('creative'),
+      unpublished: true,
     }
 
     recordTiming('theme-command:share')
-    await push(pushFlags)
+    await push(pushFlags, adminSession, multiEnvironment, context)
     recordTiming('theme-command:share')
   }
 }

packages/theme/src/cli/services/push.test.ts

@@ -15,7 +15,7 @@ import {
   promptThemeName,
   UNPUBLISHED_THEME_ROLE,
 } from '@shopify/cli-kit/node/themes/utils'
-import {renderConfirmationPrompt} from '@shopify/cli-kit/node/ui'
+import {renderConfirmationPrompt, renderError} from '@shopify/cli-kit/node/ui'
 import {AbortError} from '@shopify/cli-kit/node/error'
 import {Severity, SourceCodeType} from '@shopify/theme-check-node'
 import {outputResult} from '@shopify/cli-kit/node/output'
@@ -45,7 +45,7 @@ const adminSession = {token: '', storeFqdn: ''}
 
 describe('push', () => {
   beforeEach(() => {
-    vi.mocked(uploadTheme).mockResolvedValue({
+    vi.mocked(uploadTheme).mockReturnValue({
       workPromise: Promise.resolve(),
       uploadResults: new Map(),
       renderThemeSyncProgress: () => Promise.resolve(),
@@ -61,7 +61,7 @@ describe('push', () => {
     vi.mocked(findOrSelectTheme).mockResolvedValue(theme)
 
     // When
-    await push({...defaultFlags, publish: true})
+    await push({...defaultFlags, publish: true}, adminSession)
 
     // Then
     expect(themePublish).toHaveBeenCalledWith(theme.id, adminSession)
@@ -89,14 +89,14 @@ describe('push', () => {
       success: true,
     })
 
-    vi.mocked(uploadTheme).mockResolvedValue({
+    vi.mocked(uploadTheme).mockReturnValue({
       workPromise: Promise.resolve(),
       uploadResults,
       renderThemeSyncProgress: () => Promise.resolve(),
     })
 
     // When
-    await push({...defaultFlags, json: true})
+    await push({...defaultFlags, json: true}, adminSession)
 
     // Then
     expect(outputResult).toHaveBeenCalledWith(
@@ -129,7 +129,7 @@ describe('push', () => {
       const flags = {...defaultFlags, strict: false}
 
       // When
-      await push(flags)
+      await push(flags, adminSession)
 
       // Then
       expect(runThemeCheck).not.toHaveBeenCalled()
@@ -153,7 +153,7 @@ describe('push', () => {
       })
 
       // When/Then
-      await expect(push({...defaultFlags, strict: true})).rejects.toThrow(AbortError)
+      await expect(push({...defaultFlags, strict: true}, adminSession)).rejects.toThrow(AbortError)
     })
 
     test('blocks push when both warnings and errors exist', async () => {
@@ -183,7 +183,7 @@ describe('push', () => {
       })
 
       // When/Then
-      await expect(push({...defaultFlags, strict: true})).rejects.toThrow(AbortError)
+      await expect(push({...defaultFlags, strict: true}, adminSession)).rejects.toThrow(AbortError)
     })
 
     test('continues push when no offenses exist', async () => {
@@ -194,7 +194,7 @@ describe('push', () => {
       })
 
       // When/Then
-      await expect(push({...defaultFlags, strict: true})).resolves.not.toThrow()
+      await expect(push({...defaultFlags, strict: true}, adminSession)).resolves.not.toThrow()
     })
 
     test('continues push when only warnings exist', async () => {
@@ -215,7 +215,7 @@ describe('push', () => {
       })
 
       // When/Then
-      await expect(push({...defaultFlags, strict: true})).resolves.not.toThrow()
+      await expect(push({...defaultFlags, strict: true}, adminSession)).resolves.not.toThrow()
     })
 
     test('passes the --json flag to theme check as output format', async () => {
@@ -236,7 +236,7 @@ describe('push', () => {
       })
 
       // When/Then
-      await push({...defaultFlags, strict: true, json: true})
+      await push({...defaultFlags, strict: true, json: true}, adminSession)
       expect(runThemeCheck).toHaveBeenCalledWith(path, 'json')
     })
   })
@@ -361,4 +361,25 @@ describe('createOrSelectTheme', async () => {
     // Then
     expect(promptThemeName).toHaveBeenCalledWith('Name of the new theme')
   })
+
+  describe('when run during a multi environment command', () => {
+    test('displays error when live theme is selected without allow-live flag', async () => {
+      // Given
+      vi.mocked(findOrSelectTheme).mockResolvedValue(buildTheme({id: 3, name: 'Live Theme', role: LIVE_THEME_ROLE})!)
+      const flags: PushFlags = {live: true, environment: ['production']}
+
+      // When
+      const theme = await createOrSelectTheme(adminSession, flags, true)
+
+      // Then
+      expect(theme).toBeUndefined()
+      expect(renderError).toHaveBeenCalledWith({
+        headline: 'Environment: production',
+        body: [
+          `Can't push theme files to the live theme on ${adminSession.storeFqdn}`,
+          'Use the --allow-live flag to push to a live theme.',
+        ],
+      })
+    })
+  })
 })