-
Notifications
You must be signed in to change notification settings - Fork 449
Add typed JSON output to Hydrogen build tooling #4096
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,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 = { | ||
| 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: {}}); | ||
| }); | ||
| 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'; | ||
|
|
@@ -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, | ||
|
|
@@ -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 = { | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
@@ -157,6 +173,16 @@ export async function runBuild({ | |
| customLogger.error = (msg) => collectLog('error', msg); | ||
| } | ||
|
|
||
| if (isJsonOutput()) { | ||
|
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: overriding the logger methods covers the logs that go through
Setting |
||
| 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, | ||
|
|
@@ -203,7 +229,7 @@ export async function runBuild({ | |
| ], | ||
| }); | ||
|
|
||
| console.log(''); | ||
| if (!isJsonOutput()) console.log(''); | ||
|
|
||
| let serverBuildStatus: DeferredPromise; | ||
|
|
||
|
|
@@ -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'); | ||
|
|
||
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: this test only covers
writeJsonResultand the schema. It doesn't runBuildorrunBuild, so if thewriteJsonResultcall inBuild.runor 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 stubsrunBuildand checks thatBuild.runwritesresult.resultto stdout with--jsonwould cover the wiring.process.exitwould need stubbing too.