Skip to content
Draft
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
3 changes: 0 additions & 3 deletions packages/cli/json-output-command-exceptions.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
const commandExceptions = [
// Existing finite commands awaiting migration. Remove entries as they adopt typed JSON output.
// Do not add new finite commands to this section.
'packages/cli/src/commands/hydrogen/build.ts',
'packages/cli/src/commands/hydrogen/check.ts',
'packages/cli/src/commands/hydrogen/codegen.ts',
'packages/cli/src/commands/hydrogen/customer-account-push.ts',
'packages/cli/src/commands/hydrogen/deploy.ts',
'packages/cli/src/commands/hydrogen/env/list.ts',
Expand Down
33 changes: 30 additions & 3 deletions packages/cli/oclif.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"hydrogen:build": {
"aliases": [],
"args": {},
"description": "Builds a Hydrogen storefront for production.",
"description": "Builds a Hydrogen storefront for production. The client and app worker files are compiled to a `/dist` folder in your Hydrogen project directory.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `HydrogenBuildResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"directory\": {\n \"type\": \"string\"\n },\n \"clientDirectory\": {\n \"type\": \"string\"\n },\n \"serverDirectory\": {\n \"type\": \"string\"\n },\n \"serverFile\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"directory\",\n \"clientDirectory\",\n \"serverDirectory\",\n \"serverFile\"\n ],\n \"additionalProperties\": false,\n \"title\": \"HydrogenBuildResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```",
"flags": {
"json-schema": {
"description": "Print the command's JSON schemas.",
Expand All @@ -12,6 +12,15 @@
"allowNo": false,
"type": "boolean"
},
"json": {
"char": "j",
"description": "Output the result as JSON. Automatically disables color output.",
"env": "SHOPIFY_FLAG_JSON",
"hidden": false,
"name": "json",
"allowNo": false,
"type": "boolean"
},
"path": {
"description": "The path to the directory of the Hydrogen storefront. Defaults to the current directory where the command is run.",
"env": "SHOPIFY_HYDROGEN_FLAG_PATH",
Expand Down Expand Up @@ -117,7 +126,7 @@
"required": true
}
},
"description": "Returns diagnostic information about a Hydrogen storefront.",
"description": "Checks whether your Hydrogen app includes a set of standard Shopify routes.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `HydrogenCheckResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"missingRoutes\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"reservedRoutes\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"missingRoutes\",\n \"reservedRoutes\"\n ],\n \"additionalProperties\": false,\n \"title\": \"HydrogenCheckResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```",
"flags": {
"json-schema": {
"description": "Print the command's JSON schemas.",
Expand All @@ -126,6 +135,15 @@
"allowNo": false,
"type": "boolean"
},
"json": {
"char": "j",
"description": "Output the result as JSON. Automatically disables color output.",
"env": "SHOPIFY_FLAG_JSON",
"hidden": false,
"name": "json",
"allowNo": false,
"type": "boolean"
},
"path": {
"description": "The path to the directory of the Hydrogen storefront. Defaults to the current directory where the command is run.",
"env": "SHOPIFY_HYDROGEN_FLAG_PATH",
Expand Down Expand Up @@ -155,7 +173,7 @@
"hydrogen:codegen": {
"aliases": [],
"args": {},
"description": "Generate types for the Storefront API queries found in your project.",
"description": "Automatically generates GraphQL types for your project’s Storefront API queries.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `HydrogenCodegenResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"generatedFiles\": {\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n }\n },\n \"required\": [\n \"generatedFiles\"\n ],\n \"additionalProperties\": false,\n \"title\": \"HydrogenCodegenResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```",
"flags": {
"json-schema": {
"description": "Print the command's JSON schemas.",
Expand All @@ -164,6 +182,15 @@
"allowNo": false,
"type": "boolean"
},
"json": {
"char": "j",
"description": "Output the result as JSON. Automatically disables color output.",
"env": "SHOPIFY_FLAG_JSON",
"hidden": false,
"name": "json",
"allowNo": false,
"type": "boolean"
},
"path": {
"description": "The path to the directory of the Hydrogen storefront. Defaults to the current directory where the command is run.",
"env": "SHOPIFY_HYDROGEN_FLAG_PATH",
Expand Down
120 changes: 120 additions & 0 deletions packages/cli/src/commands/hydrogen/build-json.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import {afterEach, expect, it, vi} from 'vitest';
import {outputInfo} from '@shopify/cli-kit/node/output';
import {captureJsonOutput} from '../../../tests/output.js';
import {getRemixConfig} from '../../lib/remix-config.js';
import {codegen} from '../../lib/codegen.js';
import Check, {runCheckRoutes} from './check.js';
import Codegen, {runCodegen} from './codegen.js';
import Build from './build.js';
import {writeJsonResult} from '../../lib/json-output.js';

vi.mock('../../lib/remix-config.js', () => ({
getRemixConfig: vi.fn(),
getProjectPaths: () => ({root: '/project'}),
}));
vi.mock('../../lib/codegen.js', () => ({
codegen: vi.fn(),
spawnCodegenProcess: vi.fn(),
}));
afterEach(() => vi.restoreAllMocks());

it('encodes missing and reserved routes in one document without text banners', async () => {
vi.mocked(getRemixConfig).mockResolvedValue({
routes: {root: {id: 'root'}, cdn: {id: 'cdn', path: 'cdn/private'}},
} as any);
const {stdout, stderr} = await captureJsonOutput(() =>
runCheckRoutes({directory: '/project'}),
);
expect(JSON.parse(stdout)).toMatchObject({
missingRoutes: expect.arrayContaining(['cart']),
reservedRoutes: ['cdn/private'],
});
expect(stderr).toBe('');
});

it('encodes generated files and preserves diagnostics on stderr', async () => {
vi.mocked(getRemixConfig).mockResolvedValue({
rootDirectory: '/project',
} as any);
vi.mocked(codegen).mockImplementation(async () => {
outputInfo('Generating types');
return {'storefrontapi.generated.d.ts': ['app/**/*.tsx']};
});
const {stdout, stderr} = await captureJsonOutput(() =>
runCodegen({directory: '/project'}),
);
expect(JSON.parse(stdout)).toEqual({
generatedFiles: {'storefrontapi.generated.d.ts': ['app/**/*.tsx']},
});
expect(JSON.parse(stderr)).toMatchObject({
type: 'diagnostic',
message: 'Generating types',
});
});

it('keeps codegen failures on the fatal-error path', async () => {
vi.mocked(codegen).mockRejectedValue(new Error('Invalid query'));
const {stdout} = await captureJsonOutput(async () => {
await expect(runCodegen({directory: '/project'})).rejects.toThrow(
'Invalid query',
);
});
expect(stdout).toBe('');
});

it('encodes build output paths through the real writer', async () => {
const result = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: this test only covers writeJsonResult and the schema. It doesn't run Build or runBuild, so if the writeJsonResult call in Build.run or the Vite logger rerouting to diagnostics broke, it would still pass. The Vite build is a lot to mock, so not fussed about full coverage, but even a test that stubs runBuild and checks that Build.run writes result.result to stdout with --json would cover the wiring. process.exit would need stubbing too.

directory: '/project',
clientDirectory: '/project/dist/client',
serverDirectory: '/project/dist/server',
serverFile: '/project/dist/server/index.js',
};
const {stdout} = await captureJsonOutput(() =>
writeJsonResult(Build.jsonOutputSchema, result),
);
expect(JSON.parse(stdout)).toEqual(result);
expect(() =>
Build.jsonOutputSchema.encode({...result, serverFile: null} as any),
).toThrow();
});

it.each([Build, Codegen, Check])(
'declares JSON flags and discoverable schemas: %s',
(command) => {
expect(command.flags.json).toBeDefined();
expect(command.description).toContain(command.jsonOutputSchema.name);
},
);

it.each([Build, Codegen])(
'rejects JSON watch mode before execution: %s',
async (Command) => {
const command = new Command([], {} as any);
vi.spyOn(command as any, 'parse').mockResolvedValue({
flags: {json: true, watch: true},
});
await expect(command.run()).rejects.toThrow(
'--json cannot be combined with --watch',
);
},
);

it('rejects invalid fields and preserves empty collections', () => {
expect(() =>
Check.jsonOutputSchema.encode({
missingRoutes: [1],
reservedRoutes: [],
} as any),
).toThrow();
expect(() =>
Codegen.jsonOutputSchema.encode({generatedFiles: {types: 1}} as any),
).toThrow();
expect(
JSON.parse(
Check.jsonOutputSchema.encode({missingRoutes: [], reservedRoutes: []}),
),
).toEqual({missingRoutes: [], reservedRoutes: []});
expect(
JSON.parse(Codegen.jsonOutputSchema.encode({generatedFiles: {}})),
).toEqual({generatedFiles: {}});
});
42 changes: 37 additions & 5 deletions packages/cli/src/commands/hydrogen/build.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import {Flags} from '@oclif/core';
import Command from '@shopify/cli-kit/node/base-command';
import {resolvePath, joinPath} from '@shopify/cli-kit/node/path';
import {writeJsonResult, isJsonOutput} from '../../lib/json-output.js';
import {
flushStdout,
outputWarn,
collectLog,
outputInfo,
outputContent,
outputToken,
} from '@shopify/cli-kit/node/output';
import {emitCommandEvent} from '@shopify/cli-kit/node/command-events';
import {jsonFlag} from '@shopify/cli-kit/node/cli';
import {buildJsonOutputSchema} from '../../lib/build-tooling/types.js';
import {Flags} from '@oclif/core';
import Command from '@shopify/cli-kit/node/base-command';
import {resolvePath, joinPath} from '@shopify/cli-kit/node/path';
import {fileSize, removeFile} from '@shopify/cli-kit/node/fs';
import {getPackageManager} from '@shopify/cli-kit/node/node-package-manager';
import {commonFlags, flagsToCamelObject} from '../../lib/flags.js';
Expand Down Expand Up @@ -35,10 +40,15 @@ import {setupResourceCleanup} from '../../lib/resource-cleanup.js';
import {AbortError} from '@shopify/cli-kit/node/error';

export default class Build extends Command {
static get jsonOutputSchema(): typeof buildJsonOutputSchema {
return buildJsonOutputSchema;
}

static descriptionWithMarkdown = `Builds a Hydrogen storefront for production. The client and app worker files are compiled to a \`/dist\` folder in your Hydrogen project directory.`;

static description = 'Builds a Hydrogen storefront for production.';
static description = this.descriptionForHelp();
static flags = {
...jsonFlag,
...commonFlags.path,
...commonFlags.entry,
...commonFlags.sourcemap,
Expand All @@ -64,6 +74,10 @@ export default class Build extends Command {

async run(): Promise<void> {
const {flags} = await this.parse(Build);
if (flags.json && flags.watch)
throw new AbortError(
'--json cannot be combined with --watch. Run without --watch for a finite result.',
);
const directory = flags.path ? resolvePath(flags.path) : process.cwd();

const buildParams = {
Expand All @@ -89,6 +103,8 @@ export default class Build extends Command {
// The Remix compiler hangs due to a bug in ESBuild:
// https://github.com/evanw/esbuild/issues/2727
// The actual build has already finished so we can kill the process.
writeJsonResult(buildJsonOutputSchema, result.result, flags.json);
await flushStdout();
process.exit(0);
}
}
Expand Down Expand Up @@ -157,6 +173,16 @@ export async function runBuild({
customLogger.error = (msg) => collectLog('error', msg);
}

if (isJsonOutput()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: overriding the logger methods covers the logs that go through customLogger, but IIRC Vite's build reporter also writes progress straight to process.stdout (transforming (N) ..., rendering chunks (N)..., computing gzip size (N)... plus the matching clearLine escapes) when process.stdout.isTTY && !process.env.CI. It checks config.logLevel, not the custom logger, so that path is untouched here.

build --json | jq is fine because stdout isn't a TTY there, but anything that runs the command under a PTY (some agent harnesses, script, etc.) might end up with progress fragments before the JSON document, and parsing would fail.

Setting logLevel: 'warn' in commonConfig when isJsonOutput() should switch the reporter off, if I'm not mistaken. We'd lose the N modules transformed / chunk table diagnostics, which seems fine for JSON consumers. What do you reckon?

customLogger.info = (message) =>
emitCommandEvent({type: 'diagnostic', level: 'info', message});
customLogger.warn = (message) =>
emitCommandEvent({type: 'diagnostic', level: 'warning', message});
customLogger.warnOnce = customLogger.warn;
customLogger.error = (message) =>
emitCommandEvent({type: 'diagnostic', level: 'error', message});
}

const serverMinify = userViteConfig.build?.minify ?? true;
const commonConfig = {
root,
Expand Down Expand Up @@ -203,7 +229,7 @@ export async function runBuild({
],
});

console.log('');
if (!isJsonOutput()) console.log('');

let serverBuildStatus: DeferredPromise;

Expand Down Expand Up @@ -343,6 +369,12 @@ export async function runBuild({
}

return {
result: {
directory: root,
clientDirectory: clientOutDir,
serverDirectory: serverOutDir,
serverFile: serverOutFile,
} satisfies import('../../lib/build-tooling/types.js').BuildResult,
async close() {
codegenProcess?.removeAllListeners('close');
codegenProcess?.kill('SIGINT');
Expand Down
37 changes: 31 additions & 6 deletions packages/cli/src/commands/hydrogen/check.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import {writeJsonResult} from '../../lib/json-output.js';
import {jsonFlag} from '@shopify/cli-kit/node/cli';
import {checkJsonOutputSchema} from '../../lib/check/types.js';
import Command from '@shopify/cli-kit/node/base-command';
import {resolvePath} from '@shopify/cli-kit/node/path';
import {commonFlags} from '../../lib/flags.js';
Expand All @@ -12,12 +15,16 @@ import {
import {Args} from '@oclif/core';

export default class GenerateRoute extends Command {
static get jsonOutputSchema(): typeof checkJsonOutputSchema {
return checkJsonOutputSchema;
}

static descriptionWithMarkdown = `Checks whether your Hydrogen app includes a set of standard Shopify routes.`;

static description =
'Returns diagnostic information about a Hydrogen storefront.';
static description = this.descriptionForHelp();

static flags = {
...jsonFlag,
...commonFlags.path,
};

Expand All @@ -35,15 +42,33 @@ export default class GenerateRoute extends Command {
const directory = flags.path ? resolvePath(flags.path) : process.cwd();

if (args.resource === 'routes') {
await runCheckRoutes({directory});
await runCheckRoutes({directory}, flags.json);
} else {
throw new Error('Invalid command argument.');
}
}
}

export async function runCheckRoutes({directory}: {directory: string}) {
export async function runCheckRoutes(
options: {directory: string},
json?: boolean,
) {
const result = await checkRoutes(options);
if (!writeJsonResult(checkJsonOutputSchema, result, json)) {
logMissingRoutes(result.missingRoutes);
warnReservedRoutes(result.reservedRoutes);
}
return result;
}

export async function checkRoutes({
directory,
}: {
directory: string;
}): Promise<import('../../lib/check/types.js').CheckResult> {
const remixConfig = await getRemixConfig(directory);
logMissingRoutes(findMissingRoutes(remixConfig));
warnReservedRoutes(findReservedRoutes(remixConfig));
return {
missingRoutes: findMissingRoutes(remixConfig),
reservedRoutes: findReservedRoutes(remixConfig),
};
}
Loading
Loading