Merge pull request #6313 from Shopify/jb-multi-env-check

Allow theme check command to be called with multiple environments

Author: dejmedus

.changeset/seven-goats-agree.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': minor
+---
+
+Allow theme check command to be called with multiple environments

packages/cli/oclif.manifest.json

@@ -5121,6 +5121,9 @@
       "hiddenAliases": [
       ],
       "id": "theme:check",
+      "multiEnvironmentsFlags": [
+        "path"
+      ],
       "pluginAlias": "@shopify/cli",
       "pluginName": "@shopify/cli",
       "pluginType": "core",

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

@@ -2,17 +2,17 @@ import {themeFlags} from '../../flags.js'
 import {
   formatOffensesJson,
   formatSummary,
-  handleExit,
   initConfig,
   outputActiveChecks,
   outputActiveConfig,
   performAutoFixes,
   renderOffensesText,
   sortOffenses,
   isExtendedWriteStream,
+  handleExit,
   type FailLevel,
 } from '../../services/check.js'
-import ThemeCommand from '../../utilities/theme-command.js'
+import ThemeCommand, {RequiredFlags} from '../../utilities/theme-command.js'
 import {Flags} from '@oclif/core'
 import {globalFlags} from '@shopify/cli-kit/node/cli'
 import {outputResult, outputDebug} from '@shopify/cli-kit/node/output'
@@ -21,7 +21,10 @@ import {themeCheckRun, LegacyIdentifiers} from '@shopify/theme-check-node'
 import {findPathUp} from '@shopify/cli-kit/node/fs'
 import {moduleDirectory, joinPath} from '@shopify/cli-kit/node/path'
 import {getPackageVersion} from '@shopify/cli-kit/node/node-package-manager'
+import {InferredFlags} from '@oclif/core/interfaces'
+import {AdminSession} from '@shopify/cli-kit/node/session'
 
+type CheckFlags = InferredFlags<typeof Check.flags>
 export default class Check extends ThemeCommand {
   static summary = 'Validate the theme.'
 
@@ -86,11 +89,12 @@ export default class Check extends ThemeCommand {
     environment: themeFlags.environment,
   }
 
-  async run(): Promise<void> {
-    const {flags} = await this.parse(Check)
+  static multiEnvironmentsFlags: RequiredFlags = ['path']
 
+  async command(flags: CheckFlags, _session: AdminSession, multiEnvironment: boolean): Promise<void> {
     // Its not clear to typescript that path will always be defined
     const path = flags.path
+    const environment = flags.environment?.[0]
     // To support backwards compatibility for legacy configs
     const isLegacyConfig = flags.config?.startsWith(':') && LegacyIdentifiers.has(flags.config.slice(1))
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
@@ -121,30 +125,32 @@ export default class Check extends ThemeCommand {
     }
 
     if (flags.print) {
-      await outputActiveConfig(path, config)
+      await outputActiveConfig(path, config, environment)
 
       // --print should not trigger full theme check operation
       return
     }
 
     if (flags.list) {
-      await outputActiveChecks(path, config)
+      await outputActiveChecks(path, config, environment)
 
       // --list should not trigger full theme check operation
       return
     }
 
-    const {offenses, theme} = await runThemeCheck(path, flags.output, config)
+    const {offenses, theme} = await runThemeCheck(path, flags.output, config, environment)
 
     if (flags['auto-correct']) {
       await performAutoFixes(theme, offenses)
     }
 
-    return handleExit(offenses, flags['fail-level'] as FailLevel)
+    if (!multiEnvironment) {
+      return handleExit(offenses, flags['fail-level'] as FailLevel)
+    }
   }
 }
 
-export async function runThemeCheck(path: string, outputFormat: string, config?: string) {
+export async function runThemeCheck(path: string, outputFormat: string, config?: string, environment?: string) {
   const {offenses, theme} = await themeCheckRun(path, config, (message) => {
     if (process.env.SHOPIFY_TMP_FLAG_DEBUG) {
       outputDebug(message)
@@ -154,13 +160,13 @@ export async function runThemeCheck(path: string, outputFormat: string, config?:
   const offensesByFile = sortOffenses(offenses)
 
   if (outputFormat === 'text') {
-    renderOffensesText(offensesByFile, path)
+    renderOffensesText(offensesByFile, path, environment)
 
     // Use renderSuccess when theres no offenses
     const render = offenses.length ? renderInfo : renderSuccess
 
     render({
-      headline: 'Theme Check Summary.',
+      headline: environment ? `[${environment}] Theme Check Summary.` : 'Theme Check Summary.',
       body: formatSummary(offenses, offensesByFile, theme),
     })
   }
@@ -181,7 +187,7 @@ export async function runThemeCheck(path: string, outputFormat: string, config?:
       stdout._handle.setBlocking(true)
     }
 
-    outputResult(JSON.stringify(formatOffensesJson(offensesByFile)))
+    outputResult(JSON.stringify(formatOffensesJson(offensesByFile, environment)))
   }
 
   return {offenses, theme}

packages/theme/src/cli/services/check.ts

@@ -194,7 +194,7 @@ export function formatSummary(offenses: Offense[], offensesByFile: OffenseMap, t
   return summary
 }
 
-export function renderOffensesText(offensesByFile: OffenseMap, themeRootPath: string) {
+export function renderOffensesText(offensesByFile: OffenseMap, themeRootPath: string, environment?: string) {
   const fileNames = Object.keys(offensesByFile).sort()
 
   fileNames.forEach((filePath) => {
@@ -203,13 +203,13 @@ export function renderOffensesText(offensesByFile: OffenseMap, themeRootPath: st
     const headlineFilePath = filePath.replace(themeRootPath, '').slice(1)
 
     renderInfo({
-      headline: headlineFilePath,
+      headline: environment ? `[${environment}] ${headlineFilePath}` : headlineFilePath,
       body: formatOffenses(offensesByFile[filePath]!),
     })
   })
 }
 
-export function formatOffensesJson(offensesByFile: OffenseMap): TransformedOffenseMap[] {
+export function formatOffensesJson(offensesByFile: OffenseMap, environment?: string): TransformedOffenseMap[] {
   return Object.entries(offensesByFile).map(([path, offenses]) => {
     const transformedOffenses = offenses.map((offense: Offense) => {
       return {
@@ -226,6 +226,7 @@ export function formatOffensesJson(offensesByFile: OffenseMap): TransformedOffen
     const counts = countOffenseTypes(offenses)
 
     return {
+      environment,
       path,
       offenses: transformedOffenses,
       errorCount: counts[Severity.ERROR] ?? 0,
@@ -288,7 +289,7 @@ export async function performAutoFixes(sourceCodes: Theme, offenses: Offense[])
   await autofix(sourceCodes, offenses, saveToDiskFixApplicator)
 }
 
-export async function outputActiveConfig(themeRoot: string, configPath?: string) {
+export async function outputActiveConfig(themeRoot: string, configPath?: string, environment?: string) {
   const {ignore, settings, rootUri} = await loadConfig(configPath, themeRoot)
 
   const config = {
@@ -304,10 +305,11 @@ export async function outputActiveConfig(themeRoot: string, configPath?: string)
     // Dump out the active settings for all checks.
     ...settings,
   }
-  outputResult(YAML.stringify(config))
+  const output = environment ? {[environment]: config} : config
+  outputResult(YAML.stringify(output))
 }
 
-export async function outputActiveChecks(root: string, configPath?: string) {
+export async function outputActiveChecks(root: string, configPath?: string, environment?: string) {
   const {settings, ignore, checks} = await loadConfig(configPath, root)
   // Depending on how the configs were merged during loadConfig, there may be
   // duplicate patterns to ignore. We can clean them before outputting.
@@ -343,7 +345,8 @@ export async function outputActiveChecks(root: string, configPath?: string) {
     return acc
   }, {})
 
-  outputResult(YAML.stringify(checksList))
+  const output = environment ? {[environment]: checksList} : checksList
+  outputResult(YAML.stringify(output))
 }
 
 interface ExtendedWriteStream extends NodeJS.WriteStream {

packages/theme/src/cli/utilities/theme-command.test.ts

@@ -85,6 +85,9 @@ class TestThemeCommandWithUnionFlags extends TestThemeCommand {
     }),
   }
 }
+class TestThemeCommandWithPath extends TestThemeCommand {
+  static multiEnvironmentsFlags: RequiredFlags = ['store', 'path']
+}
 
 class TestUnauthenticatedThemeCommand extends ThemeCommand {
   static flags = {
@@ -402,6 +405,31 @@ describe('ThemeCommand', () => {
       expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['development', 'production'])
     })
 
+    test('should not execute commands in environments that are missing required flags even if they have a default value', async () => {
+      // Given
+      vi.mocked(loadEnvironment)
+        .mockResolvedValueOnce({store: 'store1.myshopify.com', path: '/a/path'})
+        .mockResolvedValueOnce({})
+        .mockResolvedValueOnce({store: 'store3.myshopify.com'})
+
+      vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
+      vi.mocked(renderConcurrent).mockResolvedValue(undefined)
+
+      await CommandConfig.load()
+      const command = new TestThemeCommandWithPath(
+        ['--environment', 'development', '--environment', 'env-missing-store', '--environment', 'path-defaults-to-cwd'],
+        CommandConfig,
+      )
+
+      // When
+      await command.run()
+
+      // Then
+      const renderConcurrentProcesses = vi.mocked(renderConcurrent).mock.calls[0]?.[0]?.processes
+      expect(renderConcurrentProcesses).toHaveLength(1)
+      expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['development'])
+    })
+
     test('should not execute commands in environments that are missing required "one of" flags', async () => {
       // Given
       vi.mocked(loadEnvironment)

packages/theme/src/cli/utilities/theme-command.ts

@@ -142,12 +142,8 @@ export default abstract class ThemeCommand extends Command {
    * @param flagsWithoutDefaults - Flags provided via the CLI
    * @returns The map of environments
    */
-  private async loadEnvironments(
-    environments: EnvironmentName[],
-    flags: FlagValues,
-    flagsWithoutDefaults: FlagValues,
-  ): Promise<Map<EnvironmentName, FlagValues>> {
-    const environmentMap = new Map<EnvironmentName, FlagValues>()
+  private async loadEnvironments(environments: EnvironmentName[], flags: FlagValues, flagsWithoutDefaults: FlagValues) {
+    const environmentMap = new Map<EnvironmentName, {flags: FlagValues; validationFlags: FlagValues}>()
 
     for (const environmentName of environments) {
       // eslint-disable-next-line no-await-in-loop
@@ -162,10 +158,13 @@ export default abstract class ThemeCommand extends Command {
       }
 
       environmentMap.set(environmentName, {
-        ...flags,
-        ...environmentFlags,
-        ...flagsWithoutDefaults,
-        environment: [environmentName],
+        flags: {
+          ...flags,
+          ...environmentFlags,
+          ...flagsWithoutDefaults,
+          environment: [environmentName],
+        },
+        validationFlags: {...environmentFlags, ...flagsWithoutDefaults} as FlagValues,
       })
     }
 
@@ -180,21 +179,21 @@ export default abstract class ThemeCommand extends Command {
    * @returns An object containing valid and invalid environment arrays
    */
   private async validateEnvironments(
-    environmentMap: Map<EnvironmentName, FlagValues>,
+    environmentMap: Map<EnvironmentName, {flags: FlagValues; validationFlags: FlagValues}>,
     requiredFlags: Exclude<RequiredFlags, null>,
     requiresAuth: boolean,
   ) {
     const valid: ValidEnvironment[] = []
     const invalid: {environment: EnvironmentName; reason: string}[] = []
 
-    for (const [environmentName, environmentFlags] of environmentMap) {
-      const validationResult = this.validConfig(environmentFlags, requiredFlags, environmentName)
+    for (const [environmentName, {flags, validationFlags}] of environmentMap) {
+      const validationResult = this.validConfig(validationFlags, requiredFlags, environmentName)
       if (validationResult !== true) {
         const missingFlagsText = validationResult.join(', ')
         invalid.push({environment: environmentName, reason: `Missing flags: ${missingFlagsText}`})
         continue
       }
-      valid.push({environment: environmentName, flags: environmentFlags, requiresAuth})
+      valid.push({environment: environmentName, flags, requiresAuth})
     }
 
     return {valid, invalid}