-
Notifications
You must be signed in to change notification settings - Fork 449
Add typed JSON output to Hydrogen maintenance commands #4103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@shopify/cli-hydrogen': minor | ||
| --- | ||
|
|
||
| Add typed JSON output and discoverable result schemas to finite Hydrogen CLI commands. Use `--json` for a machine-readable result or `--json-schema` to inspect its schema. Progress and diagnostics use JSON events on stderr, while deployment CI files and environment file updates retain their existing behavior. Build and codegen watch modes cannot be combined with `--json`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import {beforeEach, expect, it, vi} from 'vitest'; | ||
| import {captureJsonOutput} from '../../../tests/output.js'; | ||
| import {createPlatformShortcut} from '../../lib/shell.js'; | ||
| import Shortcut, {runCreateShortcut} from './shortcut.js'; | ||
| import Upgrade, {presentUpgradeResult} from './upgrade.js'; | ||
|
|
||
| vi.mock('../../lib/shell.js'); | ||
| beforeEach(() => vi.clearAllMocks()); | ||
|
|
||
| it('encodes the shortcut and shells without the success banner', async () => { | ||
| vi.mocked(createPlatformShortcut).mockResolvedValue(['zsh', 'bash']); | ||
| const {stdout, stderr} = await captureJsonOutput(() => runCreateShortcut()); | ||
| expect(JSON.parse(stdout)).toEqual({alias: 'h2', shells: ['zsh', 'bash']}); | ||
| expect(stderr).toBe(''); | ||
| }); | ||
|
|
||
| it('keeps unsupported shells on the fatal-error path in JSON mode', async () => { | ||
| vi.mocked(createPlatformShortcut).mockResolvedValue([]); | ||
| const {stdout} = await captureJsonOutput(async () => { | ||
| await expect(runCreateShortcut()).rejects.toThrow( | ||
| 'No supported shell found', | ||
| ); | ||
| }); | ||
| expect(stdout).toBe(''); | ||
| }); | ||
|
|
||
| it.each(['upgraded', 'unchanged'] as const)( | ||
| 'encodes %s results through the real upgrade presenter and writer', | ||
| async (status) => { | ||
| const result = { | ||
| status, | ||
| directory: '/project', | ||
| currentVersion: '2026.1.0', | ||
| version: '2026.4.0', | ||
| packages: ['@shopify/hydrogen@2026.4.0'], | ||
| removedPackages: ['@remix-run/react'], | ||
| instructionsFile: '.hydrogen/upgrade.md', | ||
| }; | ||
| const {stdout, stderr} = await captureJsonOutput(() => | ||
| presentUpgradeResult(result), | ||
| ); | ||
| expect(JSON.parse(stdout)).toEqual(result); | ||
| expect(stderr).toBe(''); | ||
| expect(() => | ||
| Upgrade.jsonOutputSchema.encode({...result, packages: [1]} as any), | ||
| ).toThrow(); | ||
| }, | ||
| ); | ||
|
|
||
| it.each([Shortcut, Upgrade])( | ||
| 'exposes JSON flags and discoverable schemas: %s', | ||
| (command) => { | ||
| expect(command.flags.json).toBeDefined(); | ||
| expect(command.description).toContain(command.jsonOutputSchema.name); | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| import {outputWarn} from '@shopify/cli-kit/node/output'; | ||
| import {writeJsonResult} from '../../lib/json-output.js'; | ||
| import {jsonFlag} from '@shopify/cli-kit/node/cli'; | ||
| import {upgradeJsonOutputSchema} from '../../lib/maintenance/types.js'; | ||
| import {createRequire} from 'node:module'; | ||
| import semver from 'semver'; | ||
| import cliTruncate from 'cli-truncate'; | ||
|
|
@@ -90,12 +94,17 @@ function getAllRemovedPackages(release: CumulativeRelease): string[] { | |
| const INSTRUCTIONS_FOLDER = '.hydrogen'; | ||
|
|
||
| export default class Upgrade extends Command { | ||
| static get jsonOutputSchema(): typeof upgradeJsonOutputSchema { | ||
| return upgradeJsonOutputSchema; | ||
| } | ||
|
|
||
| static descriptionWithMarkdown = | ||
| 'Upgrade Hydrogen project dependencies, preview features, fixes and breaking changes. The command also generates an instruction file for each upgrade.'; | ||
|
|
||
| static description = 'Upgrade Remix and Hydrogen npm dependencies.'; | ||
| static description = this.descriptionForHelp(); | ||
|
|
||
| static flags = { | ||
| ...jsonFlag, | ||
| ...commonFlags.path, | ||
| version: Flags.string({ | ||
| description: 'A target hydrogen version to update to', | ||
|
|
@@ -113,10 +122,13 @@ export default class Upgrade extends Command { | |
| async run(): Promise<void> { | ||
| const {flags} = await this.parse(Upgrade); | ||
|
|
||
| await runUpgrade({ | ||
| ...flagsToCamelObject(flags), | ||
| appPath: flags.path ? resolvePath(flags.path) : process.cwd(), | ||
| }); | ||
| await runUpgrade( | ||
| { | ||
| ...flagsToCamelObject(flags), | ||
| appPath: flags.path ? resolvePath(flags.path) : process.cwd(), | ||
| }, | ||
| flags.json, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -128,11 +140,39 @@ type UpgradeOptions = { | |
| force?: boolean; | ||
| }; | ||
|
|
||
| export async function runUpgrade({ | ||
| export async function runUpgrade(options: UpgradeOptions, json?: boolean) { | ||
| const {result, selectedRelease} = await executeUpgrade(options); | ||
| await presentUpgradeResult(result, selectedRelease, json); | ||
| } | ||
|
|
||
| export async function presentUpgradeResult( | ||
| result: import('../../lib/maintenance/types.js').UpgradeResult, | ||
| selectedRelease?: Release, | ||
| json?: boolean, | ||
| ) { | ||
| if (writeJsonResult(upgradeJsonOutputSchema, result, json)) return; | ||
| if (result.status === 'unchanged') { | ||
| renderSuccess({ | ||
| headline: `You are on the latest Hydrogen version: ${result.version}`, | ||
| }); | ||
| } else { | ||
| await displayUpgradeSummary({ | ||
| appPath: result.directory, | ||
| currentVersion: result.currentVersion, | ||
| selectedRelease: selectedRelease!, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocking: Let's make type UpgradeExecution =
| {result: UpgradeResult & {status: 'unchanged'}}
| {result: UpgradeResult & {status: 'upgraded'}; selectedRelease: Release};and have |
||
| instrunctionsFilePath: result.instructionsFile, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| export async function executeUpgrade({ | ||
| appPath, | ||
| version: targetVersion, | ||
| force = false, | ||
| }: UpgradeOptions) { | ||
| }: UpgradeOptions): Promise<{ | ||
| result: import('../../lib/maintenance/types.js').UpgradeResult; | ||
| selectedRelease?: Release; | ||
| }> { | ||
| // --version=next is only available when running from monorepo, tests, or CI | ||
| if (targetVersion === 'next') { | ||
| const isInTests = process.env.SHOPIFY_UNIT_TEST === '1'; | ||
|
|
@@ -188,13 +228,17 @@ export async function runUpgrade({ | |
| }); | ||
|
|
||
| if (!availableUpgrades?.length) { | ||
| renderSuccess({ | ||
| headline: `You are on the latest Hydrogen version: ${getAbsoluteVersion( | ||
| currentVersion, | ||
| )}`, | ||
| }); | ||
|
|
||
| return; | ||
| const version = getAbsoluteVersion(currentVersion); | ||
| return { | ||
| result: { | ||
| status: 'unchanged', | ||
| directory: appPath, | ||
| currentVersion: version, | ||
| version, | ||
| packages: [], | ||
| removedPackages: [], | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| let confirmed = false; | ||
|
|
@@ -254,13 +298,27 @@ export async function runUpgrade({ | |
|
|
||
| const instrunctionsFilePath = await instrunctionsFilePathPromise; | ||
|
|
||
| // Display a summary of the upgrade and next steps | ||
| await displayUpgradeSummary({ | ||
| appPath, | ||
| currentVersion, | ||
| instrunctionsFilePath, | ||
| return { | ||
| selectedRelease, | ||
| }); | ||
| result: { | ||
| status: 'upgraded', | ||
| directory: appPath, | ||
| currentVersion: getAbsoluteVersion(currentVersion), | ||
| version: getAbsoluteVersion(selectedRelease.version), | ||
| instructionsFile: instrunctionsFilePath, | ||
| packages: buildUpgradeCommandArgs({ | ||
| selectedRelease, | ||
| currentDependencies, | ||
| targetVersion, | ||
| cumulativeDependencies: cumulativeRelease.dependencies, | ||
| cumulativeDevDependencies: cumulativeRelease.devDependencies, | ||
| }), | ||
| removedPackages: [ | ||
| ...cumulativeRelease.removeDependencies, | ||
| ...cumulativeRelease.removeDevDependencies, | ||
| ].filter((name) => name in currentDependencies), | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -409,9 +467,8 @@ export async function getChangelog(): Promise<ChangeLog> { | |
| CACHED_CHANGELOG = changelog; | ||
| return changelog; | ||
| } catch (error) { | ||
| console.warn( | ||
| `Failed to load local changelog from ${localChangelogPath}:`, | ||
| (error as Error).message, | ||
| outputWarn( | ||
| `Failed to load local changelog from ${localChangelogPath}: ${(error as Error).message}`, | ||
| ); | ||
| // Fall through to remote fetch if local fails and not explicitly forced | ||
| if (process.env.FORCE_CHANGELOG_SOURCE === 'local') { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking: love that the
unchangedpath is covered end to end. Theupgradedpath is only covered with a hand-built result through the presenter though (inmaintenance-json.test.ts), so nothing checks whatexecuteUpgradeactually puts inpackages,removedPackagesandinstructionsFile. Those are the new bits of logic in this PR.Would be worth adding a JSON assertion to one of the existing real upgrade flows (e.g. the one around line 2418) so a regression in how the receipt is built gets caught.