Merge pull request #7335 from Shopify/rp/fix-theme-check-path-file-validation

fix: validate --path flag is a directory, not a file

Author: byrichardpowell

.changeset/fix-theme-check-path-file-validation.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix `ENOTDIR` error when a file path is passed to `--path` flag in theme commands. The flag now validates that the provided path is a directory and shows a helpful error message suggesting the parent directory instead.

packages/theme/src/cli/flags.test.ts

@@ -1,8 +1,8 @@
 import {themeFlags} from './flags.js'
 import {describe, expect, test} from 'vitest'
 import Command from '@shopify/cli-kit/node/base-command'
-import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs'
-import {cwd, resolvePath} from '@shopify/cli-kit/node/path'
+import {inTemporaryDirectory, writeFileSync} from '@shopify/cli-kit/node/fs'
+import {cwd, joinPath, resolvePath} from '@shopify/cli-kit/node/path'
 import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
 
 class MockCommand extends Command {
@@ -26,20 +26,34 @@ describe('themeFlags', () => {
       expect(flags.path).toEqual(cwd())
     })
 
-    test('can be expclitly provided', async () => {
+    test('can be explicitly provided', async () => {
       await inTemporaryDirectory(async (tmpDir) => {
         const flags = await MockCommand.run(['--path', tmpDir])
 
         expect(flags.path).toEqual(resolvePath(tmpDir))
       })
     })
 
-    test("renders an error message and exists when the path doesn't exist", async () => {
+    test("renders an error message and exits when the path doesn't exist", async () => {
       const mockOutput = mockAndCaptureOutput()
 
       await MockCommand.run(['--path', 'boom'])
 
       expect(mockOutput.error()).toMatch("A path was explicitly provided but doesn't exist")
     })
+
+    test('renders an error message and exits when the path is a file instead of a directory', async () => {
+      await inTemporaryDirectory(async (tmpDir) => {
+        const filePath = joinPath(tmpDir, 'section.liquid')
+        writeFileSync(filePath, '{% schema %}{% endschema %}')
+
+        const mockOutput = mockAndCaptureOutput()
+
+        await MockCommand.run(['--path', filePath])
+
+        expect(mockOutput.error()).toMatch('The path must be a directory, not a file')
+        expect(mockOutput.error()).toMatch('section.liquid')
+      })
+    })
   })
 })

packages/theme/src/cli/flags.ts

@@ -1,7 +1,7 @@
 import {Flags} from '@oclif/core'
 import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn'
-import {resolvePath, cwd} from '@shopify/cli-kit/node/path'
-import {fileExistsSync} from '@shopify/cli-kit/node/fs'
+import {resolvePath, cwd, dirname} from '@shopify/cli-kit/node/path'
+import {fileExistsSync, isDirectorySync} from '@shopify/cli-kit/node/fs'
 import {renderError} from '@shopify/cli-kit/node/ui'
 
 /**
@@ -15,17 +15,25 @@ export const themeFlags = {
     parse: async (input) => {
       const resolvedPath = resolvePath(input)
 
-      if (fileExistsSync(resolvedPath)) {
-        return resolvedPath
+      if (!fileExistsSync(resolvedPath)) {
+        // We can't use AbortError because oclif catches it and adds its own
+        // messaging that breaks our UI
+        renderError({
+          headline: "A path was explicitly provided but doesn't exist.",
+          body: [`Please check the path and try again: ${resolvedPath}`],
+        })
+        return process.exit(1)
       }
 
-      // We can't use AbortError because oclif catches it and adds its own
-      // messaging that breaks our UI
-      renderError({
-        headline: "A path was explicitly provided but doesn't exist.",
-        body: [`Please check the path and try again: ${resolvedPath}`],
-      })
-      process.exit(1)
+      if (!isDirectorySync(resolvedPath)) {
+        renderError({
+          headline: 'The path must be a directory, not a file.',
+          body: [`The provided path is not a directory: ${resolvedPath}`, `Did you mean: ${dirname(resolvedPath)}`],
+        })
+        return process.exit(1)
+      }
+
+      return resolvedPath
     },
     default: async () => cwd(),
     noCacheDefault: true,