Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-theme-check-path-file-validation.md
Original file line number Diff line number Diff line change
@@ -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.
22 changes: 18 additions & 4 deletions packages/theme/src/cli/flags.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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')
})
})
})
})
30 changes: 19 additions & 11 deletions packages/theme/src/cli/flags.ts
Original file line number Diff line number Diff line change
@@ -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'

/**
Expand All @@ -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,
Expand Down
Loading