Skip to content

Commit 5505e58

Browse files
Report Theme Check config directories as expected errors
Check up front whether the explicitly selected Theme Check config is a directory, and raise an AbortError for check, --print, and --list instead of crashing with EISDIR. The EISDIR raised by readFile often has no path, so it can't be matched after the failure like the missing-file case. Clarify in the --config flag description that it takes a file path and that --path sets the theme folder. Closes shop/issues#100399
1 parent bbde00e commit 5505e58

8 files changed

Lines changed: 86 additions & 7 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/theme': patch
3+
---
4+
5+
Report a directory passed to `theme check --config` as an expected user error instead of crashing with EISDIR.

‎docs-shopify.dev/generated/generated_docs_data_v2.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8347,7 +8347,7 @@
83478347
"syntaxKind": "PropertySignature",
83488348
"name": "-C, --config <value>",
83498349
"value": "string",
8350-
"description": "Use the config provided, overriding .theme-check.yml if present Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported",
8350+
"description": "Path to a config file to use, overriding .theme-check.yml if present Use --path to set the theme folder\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported",
83518351
"isOptional": true,
83528352
"environmentValue": "SHOPIFY_FLAG_CONFIG"
83538353
},
@@ -8388,7 +8388,7 @@
83888388
"environmentValue": "SHOPIFY_FLAG_VERSION"
83898389
}
83908390
],
8391-
"value": "export interface themecheck {\n /**\n * Alias of the Shopify account to use for authentication.\n * @environment SHOPIFY_FLAG_AUTH_ALIAS\n */\n '--auth-alias <value>'?: string\n\n /**\n * Automatically fix offenses\n * @environment SHOPIFY_FLAG_AUTO_CORRECT\n */\n '-a, --auto-correct'?: ''\n\n /**\n * Use the config provided, overriding .theme-check.yml if present\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported \n * @environment SHOPIFY_FLAG_CONFIG\n */\n '-C, --config <value>'?: string\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment <value>'?: string\n\n /**\n * Minimum severity for exit with error code\n * @environment SHOPIFY_FLAG_FAIL_LEVEL\n */\n '--fail-level <value>'?: string\n\n /**\n * Generate a .theme-check.yml file\n * @environment SHOPIFY_FLAG_INIT\n */\n '--init'?: ''\n\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * List enabled checks\n * @environment SHOPIFY_FLAG_LIST\n */\n '--list'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Disable interactive prompts and browser authentication.\n * @environment SHOPIFY_FLAG_NO_INPUT\n */\n '--no-input'?: ''\n\n /**\n * The output format to use\n * @environment SHOPIFY_FLAG_OUTPUT\n */\n '-o, --output <value>'?: string\n\n /**\n * The path where you want to run the command. Defaults to the current working directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path <value>'?: string\n\n /**\n * Output active config to STDOUT\n * @environment SHOPIFY_FLAG_PRINT\n */\n '--print'?: ''\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n\n /**\n * Print Theme Check version\n * @environment SHOPIFY_FLAG_VERSION\n */\n '-v, --version'?: ''\n}"
8391+
"value": "export interface themecheck {\n /**\n * Alias of the Shopify account to use for authentication.\n * @environment SHOPIFY_FLAG_AUTH_ALIAS\n */\n '--auth-alias <value>'?: string\n\n /**\n * Automatically fix offenses\n * @environment SHOPIFY_FLAG_AUTO_CORRECT\n */\n '-a, --auto-correct'?: ''\n\n /**\n * Path to a config file to use, overriding .theme-check.yml if present\n Use --path to set the theme folder\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported \n * @environment SHOPIFY_FLAG_CONFIG\n */\n '-C, --config <value>'?: string\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment <value>'?: string\n\n /**\n * Minimum severity for exit with error code\n * @environment SHOPIFY_FLAG_FAIL_LEVEL\n */\n '--fail-level <value>'?: string\n\n /**\n * Generate a .theme-check.yml file\n * @environment SHOPIFY_FLAG_INIT\n */\n '--init'?: ''\n\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * List enabled checks\n * @environment SHOPIFY_FLAG_LIST\n */\n '--list'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Disable interactive prompts and browser authentication.\n * @environment SHOPIFY_FLAG_NO_INPUT\n */\n '--no-input'?: ''\n\n /**\n * The output format to use\n * @environment SHOPIFY_FLAG_OUTPUT\n */\n '-o, --output <value>'?: string\n\n /**\n * The path where you want to run the command. Defaults to the current working directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path <value>'?: string\n\n /**\n * Output active config to STDOUT\n * @environment SHOPIFY_FLAG_PRINT\n */\n '--print'?: ''\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n\n /**\n * Print Theme Check version\n * @environment SHOPIFY_FLAG_VERSION\n */\n '-v, --version'?: ''\n}"
83928392
}
83938393
},
83948394
"themeconsole": {

‎packages/cli/README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12228,7 +12228,8 @@ USAGE
1222812228

1222912229
FLAGS
1223012230
-C, --config=<value>
12231-
Use the config provided, overriding .theme-check.yml if present
12231+
Path to a config file to use, overriding .theme-check.yml if present
12232+
Use --path to set the theme folder
1223212233
Supports all theme-check: config values, e.g., theme-check:theme-app-extension,
1223312234
theme-check:recommended, theme-check:all
1223412235
For backwards compatibility, :theme_app_extension is also supported

‎packages/cli/oclif.manifest.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10503,7 +10503,7 @@
1050310503
},
1050410504
"config": {
1050510505
"char": "C",
10506-
"description": "Use the config provided, overriding .theme-check.yml if present\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported ",
10506+
"description": "Path to a config file to use, overriding .theme-check.yml if present\n Use --path to set the theme folder\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported ",
1050710507
"env": "SHOPIFY_FLAG_CONFIG",
1050810508
"hasDynamicHelp": false,
1050910509
"multiple": false,

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,16 @@ describe('Check', () => {
9696
await expect(result).rejects.toThrowError(`Theme Check config file not found: ${config}`)
9797
})
9898

99+
test('reports a directory passed as the config as an expected error', async () => {
100+
await inTemporaryDirectory(async (directory) => {
101+
const result = runThemeCheck(path, 'text', directory)
102+
103+
await expect(result).rejects.toBeInstanceOf(AbortError)
104+
await expect(result).rejects.toThrowError(`Theme Check config must be a file, but ${directory} is a directory.`)
105+
expect(themeCheckRun).not.toHaveBeenCalled()
106+
})
107+
})
108+
99109
test('does not treat a missing extended config as a missing explicitly configured file', async () => {
100110
const config = '/my-theme/.theme-check.yml'
101111
const missingExtendedConfigError = Object.assign(new Error('ENOENT'), {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ export default class Check extends ThemeCommand {
5050
config: Flags.string({
5151
char: 'C',
5252
required: false,
53-
description: `Use the config provided, overriding .theme-check.yml if present
53+
description: `Path to a config file to use, overriding .theme-check.yml if present
54+
Use --path to set the theme folder
5455
Supports all theme-check: config values, e.g., theme-check:theme-app-extension,
5556
theme-check:recommended, theme-check:all
5657
For backwards compatibility, :theme_app_extension is also supported `,

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
renderOffensesText,
1010
sortOffenses,
1111
} from './check.js'
12-
import {fileExists, writeFile} from '@shopify/cli-kit/node/fs'
12+
import {fileExists, isDirectory, writeFile} from '@shopify/cli-kit/node/fs'
1313
import {AbortError} from '@shopify/cli-kit/node/error'
1414
import {outputInfo, outputSuccess} from '@shopify/cli-kit/node/output'
1515
import {renderInfo} from '@shopify/cli-kit/node/ui'
@@ -25,9 +25,14 @@ import {beforeEach, describe, expect, test, vi, type Mock} from 'vitest'
2525

2626
vi.mock('@shopify/cli-kit/node/fs', async () => ({
2727
fileExists: vi.fn(),
28+
isDirectory: vi.fn(),
2829
writeFile: vi.fn(),
2930
}))
3031

32+
beforeEach(() => {
33+
vi.mocked(isDirectory).mockResolvedValue(false)
34+
})
35+
3136
vi.mock('@shopify/cli-kit/node/output', async () => {
3237
const actual = await vi.importActual('@shopify/cli-kit/node/output')
3338
return {
@@ -449,6 +454,26 @@ describe('outputActiveConfig', () => {
449454
await expect(result).rejects.toBeInstanceOf(AbortError)
450455
await expect(result).rejects.toThrowError(`Theme Check config file not found: ${configPath}`)
451456
})
457+
458+
test('reports a directory passed as the config as an expected error', async () => {
459+
vi.mocked(fileExists).mockResolvedValue(true)
460+
vi.mocked(isDirectory).mockResolvedValue(true)
461+
462+
const result = outputActiveConfig('/my-theme', '.')
463+
464+
await expect(result).rejects.toBeInstanceOf(AbortError)
465+
await expect(result).rejects.toThrowError('Theme Check config must be a file, but . is a directory.')
466+
expect(loadConfig).not.toHaveBeenCalled()
467+
})
468+
469+
test('does not check whether modern identifiers are directories', async () => {
470+
vi.mocked(loadConfig).mockResolvedValue({settings: {}, ignore: []} as any)
471+
472+
await outputActiveConfig('/my-theme', 'theme-check:recommended')
473+
474+
expect(isDirectory).not.toHaveBeenCalled()
475+
expect(loadConfig).toHaveBeenCalledWith('theme-check:recommended', '/my-theme')
476+
})
452477
})
453478

454479
describe('outputActiveChecks', () => {
@@ -465,4 +490,24 @@ describe('outputActiveChecks', () => {
465490
await expect(result).rejects.toBeInstanceOf(AbortError)
466491
await expect(result).rejects.toThrowError(`Theme Check config file not found: ${configPath}`)
467492
})
493+
494+
test('reports a directory passed as the config as an expected error', async () => {
495+
vi.mocked(fileExists).mockResolvedValue(true)
496+
vi.mocked(isDirectory).mockResolvedValue(true)
497+
498+
const result = outputActiveChecks('/my-theme', '.')
499+
500+
await expect(result).rejects.toBeInstanceOf(AbortError)
501+
await expect(result).rejects.toThrowError('Theme Check config must be a file, but . is a directory.')
502+
expect(loadConfig).not.toHaveBeenCalled()
503+
})
504+
505+
test('does not check whether modern identifiers are directories', async () => {
506+
vi.mocked(loadConfig).mockResolvedValue({settings: {}, ignore: [], checks: []} as any)
507+
508+
await outputActiveChecks('/my-theme', 'theme-check:recommended')
509+
510+
expect(isDirectory).not.toHaveBeenCalled()
511+
expect(loadConfig).toHaveBeenCalledWith('theme-check:recommended', '/my-theme')
512+
})
468513
})

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {ThemeCheckResult} from './check/types.js'
22
import {withThemeCheckDiagnostics} from './check/diagnostics.js'
3-
import {fileExists, writeFile} from '@shopify/cli-kit/node/fs'
3+
import {fileExists, isDirectory, writeFile} from '@shopify/cli-kit/node/fs'
44
import {outputResult, outputInfo, outputSuccess, outputDebug} from '@shopify/cli-kit/node/output'
55
import {joinPath, resolvePath} from '@shopify/cli-kit/node/path'
66
import {renderInfo} from '@shopify/cli-kit/node/ui'
@@ -43,10 +43,27 @@ function isMissingThemeCheckConfigFile(error: unknown, configPath: string) {
4343
)
4444
}
4545

46+
/**
47+
* Theme Check reads the config with `readFile`, which fails with an EISDIR error that often has no `path`,
48+
* so a directory is detected before loading instead of being matched after the failure.
49+
*/
50+
async function assertThemeCheckConfigIsNotDirectory(configPath: string | undefined) {
51+
if (configPath === undefined || configPath.startsWith('theme-check:')) return
52+
// isDirectory throws on a missing path, so missing files are left to the not-found handling.
53+
if (!(await fileExists(configPath))) return
54+
if (!(await isDirectory(configPath))) return
55+
56+
throw new AbortError(
57+
`Theme Check config must be a file, but ${configPath} is a directory.`,
58+
'Pass the path to a .theme-check.yml file to --config, or omit --config to use the .theme-check.yml in your theme root. To choose the theme folder, use --path.',
59+
)
60+
}
61+
4662
async function withThemeCheckConfigErrorHandling<T>(
4763
configPath: string | undefined,
4864
operation: () => Promise<T>,
4965
): Promise<T> {
66+
await assertThemeCheckConfigIsNotDirectory(configPath)
5067
try {
5168
return await operation()
5269
} catch (error) {

0 commit comments

Comments
 (0)