diff --git a/.changeset/chilly-donkeys-exist.md b/.changeset/chilly-donkeys-exist.md new file mode 100644 index 0000000000..4739a8c050 --- /dev/null +++ b/.changeset/chilly-donkeys-exist.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': minor +--- + +Add `--strict` flag to `theme push` command, which will report `theme check` warnings and abort the operation if there are errors. diff --git a/docs-shopify.dev/commands/interfaces/theme-push.interface.ts b/docs-shopify.dev/commands/interfaces/theme-push.interface.ts index 07e48fb907..24b51d648a 100644 --- a/docs-shopify.dev/commands/interfaces/theme-push.interface.ts +++ b/docs-shopify.dev/commands/interfaces/theme-push.interface.ts @@ -78,6 +78,12 @@ export interface themepush { */ '-s, --store '?: string + /** + * Require theme check to pass without errors before pushing. Warnings are allowed. + * @environment SHOPIFY_FLAG_STRICT_PUSH + */ + '--strict'?: '' + /** * Theme ID or name of the remote theme. * @environment SHOPIFY_FLAG_THEME_ID diff --git a/docs-shopify.dev/generated/generated_docs_data.json b/docs-shopify.dev/generated/generated_docs_data.json index afc8958649..9ded999b8e 100644 --- a/docs-shopify.dev/generated/generated_docs_data.json +++ b/docs-shopify.dev/generated/generated_docs_data.json @@ -5819,6 +5819,15 @@ "isOptional": true, "environmentValue": "SHOPIFY_FLAG_PATH" }, + { + "filePath": "docs-shopify.dev/commands/interfaces/theme-push.interface.ts", + "syntaxKind": "PropertySignature", + "name": "--strict", + "value": "\"\"", + "description": "Require theme check to pass without errors before pushing. Warnings are allowed.", + "isOptional": true, + "environmentValue": "SHOPIFY_FLAG_STRICT_PUSH" + }, { "filePath": "docs-shopify.dev/commands/interfaces/theme-push.interface.ts", "syntaxKind": "PropertySignature", @@ -5937,7 +5946,7 @@ "environmentValue": "SHOPIFY_FLAG_IGNORE" } ], - "value": "export interface themepush {\n /**\n * Allow push to a live theme.\n * @environment SHOPIFY_FLAG_ALLOW_LIVE\n */\n '-a, --allow-live'?: ''\n\n /**\n * Push theme files from your remote development theme.\n * @environment SHOPIFY_FLAG_DEVELOPMENT\n */\n '-d, --development'?: ''\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Skip downloading the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_IGNORE\n */\n '-x, --ignore '?: string\n\n /**\n * Output the result as JSON.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Push theme files from your remote live theme.\n * @environment SHOPIFY_FLAG_LIVE\n */\n '-l, --live'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Prevent deleting remote files that don't exist locally.\n * @environment SHOPIFY_FLAG_NODELETE\n */\n '-n, --nodelete'?: ''\n\n /**\n * Download only the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_ONLY\n */\n '-o, --only '?: string\n\n /**\n * Password generated from the Theme Access app.\n * @environment SHOPIFY_CLI_THEME_TOKEN\n */\n '--password '?: string\n\n /**\n * The path to your theme directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Publish as the live theme after uploading.\n * @environment SHOPIFY_FLAG_PUBLISH\n */\n '-p, --publish'?: ''\n\n /**\n * Store URL. It can be the store prefix (example) or the full myshopify.com URL (example.myshopify.com, https://example.myshopify.com).\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * Theme ID or name of the remote theme.\n * @environment SHOPIFY_FLAG_THEME_ID\n */\n '-t, --theme '?: string\n\n /**\n * Create a new unpublished theme and push to it.\n * @environment SHOPIFY_FLAG_UNPUBLISHED\n */\n '-u, --unpublished'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" + "value": "export interface themepush {\n /**\n * Allow push to a live theme.\n * @environment SHOPIFY_FLAG_ALLOW_LIVE\n */\n '-a, --allow-live'?: ''\n\n /**\n * Push theme files from your remote development theme.\n * @environment SHOPIFY_FLAG_DEVELOPMENT\n */\n '-d, --development'?: ''\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Skip downloading the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_IGNORE\n */\n '-x, --ignore '?: string\n\n /**\n * Output the result as JSON.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Push theme files from your remote live theme.\n * @environment SHOPIFY_FLAG_LIVE\n */\n '-l, --live'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Prevent deleting remote files that don't exist locally.\n * @environment SHOPIFY_FLAG_NODELETE\n */\n '-n, --nodelete'?: ''\n\n /**\n * Download only the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_ONLY\n */\n '-o, --only '?: string\n\n /**\n * Password generated from the Theme Access app.\n * @environment SHOPIFY_CLI_THEME_TOKEN\n */\n '--password '?: string\n\n /**\n * The path to your theme directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Publish as the live theme after uploading.\n * @environment SHOPIFY_FLAG_PUBLISH\n */\n '-p, --publish'?: ''\n\n /**\n * Store URL. It can be the store prefix (example) or the full myshopify.com URL (example.myshopify.com, https://example.myshopify.com).\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * Require theme check to pass without errors before pushing. Warnings are allowed.\n * @environment SHOPIFY_FLAG_STRICT_PUSH\n */\n '--strict'?: ''\n\n /**\n * Theme ID or name of the remote theme.\n * @environment SHOPIFY_FLAG_THEME_ID\n */\n '-t, --theme '?: string\n\n /**\n * Create a new unpublished theme and push to it.\n * @environment SHOPIFY_FLAG_UNPUBLISHED\n */\n '-u, --unpublished'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } } } diff --git a/packages/cli/README.md b/packages/cli/README.md index bcac5dbbde..5d066bf40b 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -2118,6 +2118,7 @@ FLAGS --no-color Disable color output. --password= Password generated from the Theme Access app. --path= The path to your theme directory. + --strict Require theme check to pass without errors before pushing. Warnings are allowed. --verbose Increase the verbosity of the output. DESCRIPTION diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index cfdf8c26b4..de76f74ef4 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -5791,6 +5791,13 @@ "name": "store", "type": "option" }, + "strict": { + "allowNo": false, + "description": "Require theme check to pass without errors before pushing. Warnings are allowed.", + "env": "SHOPIFY_FLAG_STRICT_PUSH", + "name": "strict", + "type": "boolean" + }, "theme": { "char": "t", "description": "Theme ID or name of the remote theme.", diff --git a/packages/theme/src/cli/commands/theme/check.ts b/packages/theme/src/cli/commands/theme/check.ts index 32cbd5c30a..771b80b0f4 100644 --- a/packages/theme/src/cli/commands/theme/check.ts +++ b/packages/theme/src/cli/commands/theme/check.ts @@ -134,52 +134,58 @@ export default class Check extends ThemeCommand { return } - const {offenses, theme} = await themeCheckRun(path, config, (message) => { - // We should replace this with outputDebug when it logs to STDERR by default. - // We need this right now because the --verbose flags pollutes STDOUT - // when used with --output=json with the current outputDebug defaults. - if (process.env.SHOPIFY_TMP_FLAG_DEBUG) { - consoleError(message) - } - }) - - const offensesByFile = sortOffenses(offenses) + const {offenses, theme} = await runThemeCheck(path, flags.output, config) - if (flags.output === 'text') { - renderOffensesText(offensesByFile, path) + if (flags['auto-correct']) { + await performAutoFixes(theme, offenses) + } - // Use renderSuccess when theres no offenses - const render = offenses.length ? renderInfo : renderSuccess + return handleExit(offenses, flags['fail-level'] as FailLevel) + } +} - render({ - headline: 'Theme Check Summary.', - body: formatSummary(offenses, offensesByFile, theme), - }) +export async function runThemeCheck(path: string, outputFormat: string, config?: string) { + const {offenses, theme} = await themeCheckRun(path, config, (message) => { + // We should replace this with outputDebug when it logs to STDERR by default. + // We need this right now because the --verbose flags pollutes STDOUT + // when used with --output=json with the current outputDebug defaults. + if (process.env.SHOPIFY_TMP_FLAG_DEBUG) { + consoleError(message) } + }) - if (flags.output === 'json') { - /** - * Workaround: - * Force stdout to be blocking so that the JSON output is not broken when piped to another process. - * ie: ` | jq .` - * It turns out that console.log is technically asynchronous, and when we call process.exit(), - * node doesn't wait on all the output being sent to stdout and instead closes the process immediately - * - * https://github.com/pnp/cli-microsoft365/issues/1266#issuecomment-727254264 - * - */ - const stdout = process.stdout - if (isExtendedWriteStream(stdout)) { - stdout._handle.setBlocking(true) - } + const offensesByFile = sortOffenses(offenses) - outputInfo(JSON.stringify(formatOffensesJson(offensesByFile))) - } + if (outputFormat === 'text') { + renderOffensesText(offensesByFile, path) - if (flags['auto-correct']) { - await performAutoFixes(theme, offenses) + // Use renderSuccess when theres no offenses + const render = offenses.length ? renderInfo : renderSuccess + + render({ + headline: 'Theme Check Summary.', + body: formatSummary(offenses, offensesByFile, theme), + }) + } + + if (outputFormat === 'json') { + /** + * Workaround: + * Force stdout to be blocking so that the JSON output is not broken when piped to another process. + * ie: ` | jq .` + * It turns out that console.log is technically asynchronous, and when we call process.exit(), + * node doesn't wait on all the output being sent to stdout and instead closes the process immediately + * + * https://github.com/pnp/cli-microsoft365/issues/1266#issuecomment-727254264 + * + */ + const stdout = process.stdout + if (isExtendedWriteStream(stdout)) { + stdout._handle.setBlocking(true) } - return handleExit(offenses, flags['fail-level'] as FailLevel) + outputInfo(JSON.stringify(formatOffensesJson(offensesByFile))) } + + return {offenses, theme} } diff --git a/packages/theme/src/cli/commands/theme/push.ts b/packages/theme/src/cli/commands/theme/push.ts index e2925507a7..c7c3f55fd4 100644 --- a/packages/theme/src/cli/commands/theme/push.ts +++ b/packages/theme/src/cli/commands/theme/push.ts @@ -97,6 +97,10 @@ export default class Push extends ThemeCommand { description: 'Proceed without confirmation, if current directory does not seem to be theme directory.', env: 'SHOPIFY_FLAG_FORCE', }), + strict: Flags.boolean({ + description: 'Require theme check to pass without errors before pushing. Warnings are allowed.', + env: 'SHOPIFY_FLAG_STRICT_PUSH', + }), } async run(): Promise { @@ -120,6 +124,7 @@ export default class Push extends ThemeCommand { force: flags.force, noColor: flags['no-color'], verbose: flags.verbose, + strict: flags.strict, } await push(pushFlags) diff --git a/packages/theme/src/cli/services/push.test.ts b/packages/theme/src/cli/services/push.test.ts index eae7a6dfd2..c930319a7a 100644 --- a/packages/theme/src/cli/services/push.test.ts +++ b/packages/theme/src/cli/services/push.test.ts @@ -4,6 +4,7 @@ import {setDevelopmentTheme} from './local-storage.js' import {uploadTheme} from '../utilities/theme-uploader.js' import {ensureThemeStore} from '../utilities/theme-store.js' import {findOrSelectTheme} from '../utilities/theme-selector.js' +import {runThemeCheck} from '../commands/theme/check.js' import {buildTheme} from '@shopify/cli-kit/node/themes/factories' import {test, describe, vi, expect, beforeEach} from 'vitest' import {createTheme, fetchTheme, themePublish} from '@shopify/cli-kit/node/themes/api' @@ -15,6 +16,8 @@ import { UNPUBLISHED_THEME_ROLE, } from '@shopify/cli-kit/node/themes/utils' import {renderConfirmationPrompt} from '@shopify/cli-kit/node/ui' +import {AbortError} from '@shopify/cli-kit/node/error' +import {Severity, SourceCodeType} from '@shopify/theme-check-node' vi.mock('../utilities/theme-uploader.js') vi.mock('../utilities/theme-store.js') @@ -24,6 +27,7 @@ vi.mock('@shopify/cli-kit/node/themes/utils') vi.mock('@shopify/cli-kit/node/session') vi.mock('@shopify/cli-kit/node/themes/api') vi.mock('@shopify/cli-kit/node/ui') +vi.mock('../commands/theme/check.js') const path = '/my-theme' const defaultFlags: PullFlags = { @@ -59,6 +63,129 @@ describe('push', () => { // Then expect(themePublish).toHaveBeenCalledWith(theme.id, adminSession) }) + + describe('strict mode', () => { + beforeEach(() => { + const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! + vi.mocked(findOrSelectTheme).mockResolvedValue(theme) + }) + + test('skips theme check when strict mode is disabled', async () => { + // Given + const flags = {...defaultFlags, strict: false} + + // When + await push(flags) + + // Then + expect(runThemeCheck).not.toHaveBeenCalled() + }) + + test('blocks push when errors exist', async () => { + // Given + vi.mocked(runThemeCheck).mockResolvedValue({ + offenses: [ + { + severity: Severity.ERROR, + message: 'error message', + type: SourceCodeType.LiquidHtml, + check: 'check', + absolutePath: '/path/to/file.liquid', + start: {index: 0, line: 1, character: 1}, + end: {index: 1, line: 1, character: 1}, + }, + ], + theme: [], + }) + + // When/Then + await expect(push({...defaultFlags, strict: true})).rejects.toThrow(AbortError) + }) + + test('blocks push when both warnings and errors exist', async () => { + // Given + vi.mocked(runThemeCheck).mockResolvedValue({ + offenses: [ + { + severity: Severity.WARNING, + message: 'warning message', + type: SourceCodeType.LiquidHtml, + check: 'check', + absolutePath: '/path/to/file.liquid', + start: {index: 0, line: 1, character: 1}, + end: {index: 1, line: 1, character: 1}, + }, + { + severity: Severity.ERROR, + message: 'error message', + type: SourceCodeType.LiquidHtml, + check: 'check', + absolutePath: '/path/to/file.liquid', + start: {index: 0, line: 1, character: 1}, + end: {index: 1, line: 1, character: 1}, + }, + ], + theme: [], + }) + + // When/Then + await expect(push({...defaultFlags, strict: true})).rejects.toThrow(AbortError) + }) + + test('continues push when no offenses exist', async () => { + // Given + vi.mocked(runThemeCheck).mockResolvedValue({ + offenses: [], + theme: [], + }) + + // When/Then + await expect(push({...defaultFlags, strict: true})).resolves.not.toThrow() + }) + + test('continues push when only warnings exist', async () => { + // Given + vi.mocked(runThemeCheck).mockResolvedValue({ + offenses: [ + { + severity: Severity.WARNING, + message: 'warning message', + check: 'check', + absolutePath: '/path/to/file.liquid', + type: SourceCodeType.LiquidHtml, + start: {index: 0, line: 1, character: 1}, + end: {index: 1, line: 1, character: 1}, + }, + ], + theme: [], + }) + + // When/Then + await expect(push({...defaultFlags, strict: true})).resolves.not.toThrow() + }) + + test('passes the --json flag to theme check as output format', async () => { + // Given + vi.mocked(runThemeCheck).mockResolvedValue({ + offenses: [ + { + severity: Severity.WARNING, + message: 'warning message', + check: 'check', + absolutePath: '/path/to/file.liquid', + type: SourceCodeType.LiquidHtml, + start: {index: 0, line: 1, character: 1}, + end: {index: 1, line: 1, character: 1}, + }, + ], + theme: [], + }) + + // When/Then + await push({...defaultFlags, strict: true, json: true}) + expect(runThemeCheck).toHaveBeenCalledWith(path, 'json') + }) + }) }) describe('createOrSelectTheme', async () => { diff --git a/packages/theme/src/cli/services/push.ts b/packages/theme/src/cli/services/push.ts index 2936c6fc83..b5fb95f33d 100644 --- a/packages/theme/src/cli/services/push.ts +++ b/packages/theme/src/cli/services/push.ts @@ -7,6 +7,7 @@ import {DevelopmentThemeManager} from '../utilities/development-theme-manager.js import {findOrSelectTheme} from '../utilities/theme-selector.js' import {Role} from '../utilities/theme-selector/fetch.js' import {configureCLIEnvironment} from '../utilities/cli-config.js' +import {runThemeCheck} from '../commands/theme/check.js' import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' import {createTheme, fetchChecksums, themePublish} from '@shopify/cli-kit/node/themes/api' import {Result, Theme} from '@shopify/cli-kit/node/themes/types' @@ -20,6 +21,8 @@ import { import {themeEditorUrl, themePreviewUrl} from '@shopify/cli-kit/node/themes/urls' import {cwd, resolvePath} from '@shopify/cli-kit/node/path' import {LIVE_THEME_ROLE, promptThemeName, UNPUBLISHED_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils' +import {AbortError} from '@shopify/cli-kit/node/error' +import {Severity} from '@shopify/theme-check-node' interface PushOptions { path: string @@ -94,6 +97,9 @@ export interface PushFlags { /** Increase the verbosity of the output. */ verbose?: boolean + + /** Require theme check to pass without errors before pushing. Warnings are allowed. */ + strict?: boolean } /** @@ -102,6 +108,18 @@ export interface PushFlags { * @param flags - The flags for the push operation. */ export async function push(flags: PushFlags): Promise { + if (flags.strict) { + const outputType = flags.json ? 'json' : 'text' + const {offenses} = await runThemeCheck(flags.path ?? cwd(), outputType) + + if (offenses.length > 0) { + const errorOffenses = offenses.filter((offense) => offense.severity === Severity.ERROR) + if (errorOffenses.length > 0) { + throw new AbortError('Theme check failed. Please fix the errors before pushing.') + } + } + } + const {path} = flags configureCLIEnvironment({