Show a warning when there are multiple CLI installations

.changeset/moody-beds-sneeze.md

@@ -0,0 +1,6 @@
+---
+'@shopify/cli-kit': patch
+'@shopify/app': patch
+---
+
+Show a warning when there are multiple CLI installations

packages/app/src/cli/models/app/loader.test.ts

@@ -27,21 +27,25 @@ import {
   pnpmLockfile,
   PackageJson,
   pnpmWorkspaceFile,
+  localCLIVersion,
 } from '@shopify/cli-kit/node/node-package-manager'
 import {inTemporaryDirectory, moveFile, mkdir, mkTmpDir, rmdir, writeFile} from '@shopify/cli-kit/node/fs'
 import {joinPath, dirname, cwd, normalizePath} from '@shopify/cli-kit/node/path'
 import {platformAndArch} from '@shopify/cli-kit/node/os'
 import {outputContent, outputToken} from '@shopify/cli-kit/node/output'
 import {zod} from '@shopify/cli-kit/node/schema'
 import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
-import {currentProcessIsGlobal} from '@shopify/cli-kit/node/is-global'
 import colors from '@shopify/cli-kit/node/colors'
-// eslint-disable-next-line no-restricted-imports
-import {resolve} from 'path'
+
+import {globalCLIVersion, isGlobalCLIInstalled} from '@shopify/cli-kit/node/is-global'
 
 vi.mock('../../services/local-storage.js')
 vi.mock('../../services/app/config/use.js')
 vi.mock('@shopify/cli-kit/node/is-global')
+vi.mock('@shopify/cli-kit/node/node-package-manager', async () => ({
+  ...((await vi.importActual('@shopify/cli-kit/node/node-package-manager')) as any),
+  localCLIVersion: vi.fn(),
+}))
 
 describe('load', () => {
   let specifications: ExtensionSpecification[] = []
@@ -322,9 +326,10 @@ wrong = "property"
 
   test('shows warning if using global CLI but app has local dependency', async () => {
     // Given
-    vi.mocked(currentProcessIsGlobal).mockReturnValueOnce(true)
+    vi.mocked(isGlobalCLIInstalled).mockResolvedValue(true)
+    vi.mocked(globalCLIVersion).mockResolvedValue('3.68.0')
+    vi.mocked(localCLIVersion).mockResolvedValue('3.65.0')
     const mockOutput = mockAndCaptureOutput()
-    mockOutput.clear()
     await writeConfig(appConfiguration, {
       workspaces: ['packages/*'],
       name: 'my_app',
@@ -339,25 +344,28 @@ wrong = "property"
     expect(mockOutput.info()).toMatchInlineSnapshot(`
       "╭─ info ───────────────────────────────────────────────────────────────────────╮
       │                                                                              │
-      │  You are running a global installation of Shopify CLI                        │
+      │  Two Shopify CLI installations found – using local dependency                │
       │                                                                              │
-      │  This project has Shopify CLI as a local dependency in package.json. If you  │
-      │   prefer to use that version, run the command with your package manager      │
-      │  (e.g. npm run shopify).                                                     │
+      │  A global installation (v3.68.0) and a local dependency (v3.65.0) were       │
+      │  detected.                                                                   │
+      │  We recommend removing the @shopify/cli and @shopify/app dependencies from   │
+      │  your package.json, unless you want to use different versions across         │
+      │  multiple apps.                                                              │
       │                                                                              │
-      │  For more information, see Shopify CLI documentation [1]                     │
+      │  See Shopify CLI documentation. [1]                                          │
       │                                                                              │
       ╰──────────────────────────────────────────────────────────────────────────────╯
-      [1] https://shopify.dev/docs/apps/tools/cli
+      [1] https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executab
+      le-or-local-dependency
       "
     `)
     mockOutput.clear()
   })
 
   test('doesnt show warning if there is no local dependency', async () => {
     // Given
+    vi.mocked(localCLIVersion).mockResolvedValue(undefined)
     const mockOutput = mockAndCaptureOutput()
-    mockOutput.clear()
     await writeConfig(appConfiguration, {
       workspaces: ['packages/*'],
       name: 'my_app',
@@ -2325,7 +2333,7 @@ wrong = "property"
 
       // Then
       expect(use).toHaveBeenCalledWith({
-        directory: normalizePath(resolve(tmpDir)),
+        directory: normalizePath(tmpDir),
         shouldRenderSuccess: false,
         warningContent: {
           headline: "Couldn't find shopify.app.non-existent.toml",

packages/app/src/cli/models/app/loader.ts

@@ -37,6 +37,7 @@ import {
   getPackageManager,
   getPackageName,
   usesWorkspaces as appUsesWorkspaces,
+  localCLIVersion,
 } from '@shopify/cli-kit/node/node-package-manager'
 import {resolveFramework} from '@shopify/cli-kit/node/framework'
 import {hashString} from '@shopify/cli-kit/node/crypto'
@@ -48,7 +49,7 @@ import {joinWithAnd, slugify} from '@shopify/cli-kit/common/string'
 import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array'
 import {checkIfIgnoredInGitRepository} from '@shopify/cli-kit/node/git'
 import {renderInfo} from '@shopify/cli-kit/node/ui'
-import {currentProcessIsGlobal} from '@shopify/cli-kit/node/is-global'
+import {currentProcessIsGlobal, globalCLIVersion} from '@shopify/cli-kit/node/is-global'
 
 const defaultExtensionDirectory = 'extensions/*'
 
@@ -304,7 +305,7 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
     const name = await loadAppName(directory)
     const nodeDependencies = await getDependencies(packageJSONPath)
     const packageManager = await getPackageManager(directory)
-    this.showGlobalCLIWarningIfNeeded(nodeDependencies, packageManager)
+    await this.showMultipleCLIWarningIfNeeded(directory, nodeDependencies)
     const {webs, usedCustomLayout: usedCustomLayoutForWeb} = await this.loadWebs(
       directory,
       configuration.web_directories,
@@ -352,21 +353,28 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
     return parseConfigurationFile(schema, filepath, this.abortOrReport.bind(this), decode)
   }
 
-  private showGlobalCLIWarningIfNeeded(nodeDependencies: {[key: string]: string}, packageManager: string) {
-    const hasLocalCLI = nodeDependencies['@shopify/cli'] !== undefined
-    // Show the warning IFF:
-    // - The current process is global
-    // - The project has a local CLI
+  private async showMultipleCLIWarningIfNeeded(directory: string, dependencies: {[key: string]: string}) {
+    // Show the warning if:
+    // - There is a global installation
+    // - The project has a local CLI dependency
     // - The user didn't include the --json flag (to avoid showing the warning in scripts or CI/CD pipelines)
-    if (currentProcessIsGlobal() && hasLocalCLI && !sniffForJson() && !alreadyShownCLIWarning) {
+    // - The warning hasn't been shown yet during the current command execution
+
+    const localVersion = dependencies['@shopify/cli'] && (await localCLIVersion(directory))
+    const globalVersion = await globalCLIVersion()
+
+    if (localVersion && globalVersion && !sniffForJson() && !alreadyShownCLIWarning) {
+      const currentInstallation = currentProcessIsGlobal() ? 'global installation' : 'local dependency'
+
       const warningContent = {
-        headline: 'You are running a global installation of Shopify CLI',
+        headline: `Two Shopify CLI installations found – using ${currentInstallation}`,
         body: [
-          `This project has Shopify CLI as a local dependency in package.json. If you prefer to use that version, run the command with your package manager (e.g. ${packageManager} run shopify).`,
+          `A global installation (v${globalVersion}) and a local dependency (v${localVersion}) were detected.
+We recommend removing the @shopify/cli and @shopify/app dependencies from your package.json, unless you want to use different versions across multiple apps.`,
         ],
         link: {
-          label: 'For more information, see Shopify CLI documentation',
-          url: 'https://shopify.dev/docs/apps/tools/cli',
+          label: 'See Shopify CLI documentation.',
+          url: 'https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executable-or-local-dependency',
         },
       }
       renderInfo(warningContent)
@@ -397,12 +405,12 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
   private validateWebs(webs: Web[]): void {
     ;[WebType.Backend, WebType.Frontend].forEach((webType) => {
       const websOfType = webs.filter((web) => web.configuration.roles.includes(webType))
-      if (websOfType.length > 1) {
+      if (websOfType[1]) {
         this.abortOrReport(
           outputContent`You can only have one web with the ${outputToken.yellow(webType)} role in your app`,
           undefined,
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          joinPath(websOfType[1]!.directory, configurationFileNames.web),
+
+          joinPath(websOfType[1].directory, configurationFileNames.web),
         )
       }
     })
@@ -1048,12 +1056,12 @@ async function getProjectType(webs: Web[]): Promise<'node' | 'php' | 'ruby' | 'f
     return
   } else if (backendWebs.length === 0 && frontendWebs.length > 0) {
     return 'frontend'
-  } else if (backendWebs.length === 0) {
+  } else if (!backendWebs[0]) {
     outputDebug('Unable to decide project type as no web backend')
     return
   }
-  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-  const {directory} = backendWebs[0]!
+
+  const {directory} = backendWebs[0]
 
   const nodeConfigFile = joinPath(directory, 'package.json')
   const rubyConfigFile = joinPath(directory, 'Gemfile')

packages/cli-kit/src/public/node/is-global.test.ts

@@ -1,5 +1,6 @@
 import {
   currentProcessIsGlobal,
+  globalCLIVersion,
   inferPackageManagerForGlobalCLI,
   installGlobalCLIPrompt,
   isGlobalCLIInstalled,
@@ -130,6 +131,41 @@ describe('isGlobalCLIInstalled', () => {
   })
 })
 
+describe('globalCLIVersion', () => {
+  test('returns the version when a recent CLI is installed globally', async () => {
+    // Given
+    vi.mocked(captureOutput).mockImplementationOnce(() => Promise.resolve('3.65.0'))
+
+    // When
+    const got = await globalCLIVersion()
+
+    // Then
+    expect(got).toBe('3.65.0')
+  })
+
+  test('returns undefined when the global version is older than 3.59', async () => {
+    // Given
+    vi.mocked(captureOutput).mockImplementationOnce(() => Promise.resolve('3.55.0'))
+
+    // When
+    const got = await globalCLIVersion()
+
+    // Then
+    expect(got).toBeUndefined()
+  })
+
+  test('returns undefined when the global version is not installed', async () => {
+    // Given
+    vi.mocked(captureOutput).mockImplementationOnce(() => Promise.resolve('command not found: shopify'))
+
+    // When
+    const got = await globalCLIVersion()
+
+    // Then
+    expect(got).toBeUndefined()
+  })
+})
+
 describe('installGlobalCLIPrompt', () => {
   test('if global CLI is already installed', async () => {
     // Given

packages/cli-kit/src/public/node/is-global.ts

@@ -1,5 +1,5 @@
 import {isUnitTest} from './context/local.js'
-import {PackageManager} from './node-package-manager.js'
+import {PackageManager, versionSatisfies} from './node-package-manager.js'
 import {outputInfo} from './output.js'
 import {cwd, sniffForPath} from './path.js'
 import {captureOutput, exec, terminalSupportsPrompting} from './system.js'
@@ -51,6 +51,24 @@ export async function isGlobalCLIInstalled(): Promise<boolean> {
   }
 }
 
+/**
+ * Returns the version of the globally installed CLI, only if it's greater than 3.59.0 (when the global CLI was introduced).
+ *
+ * @returns The version of the CLI if it is globally installed or undefined.
+ */
+export async function globalCLIVersion(): Promise<string | undefined> {
+  try {
+    const version = await captureOutput('shopify', ['version'])
+    if (versionSatisfies(version, `>=3.59.0`)) {
+      return version
+    }
+    return undefined
+    // eslint-disable-next-line no-catch-all/no-catch-all
+  } catch {
+    return undefined
+  }
+}
+
 /**
  * Installs the global Shopify CLI, using the provided package manager.
  *

packages/cli-kit/src/public/node/node-package-manager.test.ts

@@ -20,6 +20,7 @@ import {
   checkForCachedNewVersion,
   inferPackageManager,
   PackageManager,
+  localCLIVersion,
 } from './node-package-manager.js'
 import {captureOutput, exec} from './system.js'
 import {inTemporaryDirectory, mkdir, touchFile, writeFile} from './fs.js'
@@ -1027,3 +1028,33 @@ describe('inferPackageManager', () => {
     expect(inferPackageManager(undefined, mockEnv)).toBe('npm')
   })
 })
+
+describe('localCLIVersion', () => {
+  test('returns the version of the local CLI', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      vi.mocked(captureOutput).mockResolvedValueOnce(`folder@ ${tmpDir}
+└── @shopify/cli@3.68.0`)
+
+      // When
+      const got = await localCLIVersion(tmpDir)
+
+      // Then
+      expect(got).toEqual('3.68.0')
+    })
+  })
+
+  test('returns undefined when the dependency is not installed', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      vi.mocked(captureOutput).mockResolvedValueOnce(`folder@ ${tmpDir}
+        └── (empty)`)
+
+      // When
+      const got = await localCLIVersion(tmpDir)
+
+      // Then
+      expect(got).toBeUndefined()
+    })
+  })
+})

packages/cli-kit/src/public/node/node-package-manager.ts

@@ -732,3 +732,19 @@ export function inferPackageManager(optionsPackageManager: string | undefined, e
 
   return 'npm'
 }
+
+/**
+ * Returns the version of the local dependency of the CLI if it's installed in the provided directory.
+ *
+ * @param directory - path of the project to look for the dependency.
+ * @returns the CLI version or undefined if the dependency is not installed.
+ */
+export async function localCLIVersion(directory: string): Promise<string | undefined> {
+  try {
+    const output = await captureOutput('npm', ['list', '@shopify/cli'], {cwd: directory})
+    return output.match(/@shopify\/cli@([\w.-]*)/)?.[1]
+    // eslint-disable-next-line no-catch-all/no-catch-all
+  } catch {
+    return undefined
+  }
+}