Repository navigation
Add typed JSON output to app build #8801
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
11dbcff
Add typed JSON output to app build
isaacroldan decd92f
Document JSON interactive UI routing with app build
isaacroldan 3565db9
Keep reached config recovery and compiler output on JSON diagnostics
isaacroldan db07cf8
Prove reached build recovery and discoverable directory paths
isaacroldan 38c9ffa
Declare the compiler subprocess fixture as a used test dependency
isaacroldan faa817d
Refresh build directory schema discovery
isaacroldan 7edbe5a
Consolidate app build recovery tests and separate shared UI changes
isaacroldan da60bc9
Report only the app build success status in JSON
isaacroldan 05ca236
Simplify app build test mocks
isaacroldan 3169aac
Remove the compiler pipeline integration fixture
isaacroldan 68234d4
Address app build JSON review findings
isaacroldan 49f303e
Accept static fields in command JSON schema lint rule
isaacroldan bfc346d
Simplify app build JSON tests and schema declaration
isaacroldan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@shopify/cli': minor | ||
| --- | ||
|
|
||
| Add JSON status output to app build. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
87 changes: 87 additions & 0 deletions
87
packages/app/src/cli/commands/app/build.context-recovery.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import Build from './build.js' | ||
| import build from '../../services/build.js' | ||
| import * as localStorage from '../../services/local-storage.js' | ||
| import {LocalStorage} from '@shopify/cli-kit/node/local-storage' | ||
| import {runWithCommandEventsForCommand} from '@shopify/cli-kit/node/command-events' | ||
| import {withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output' | ||
| import {inTemporaryDirectory, readFile, writeFile} from '@shopify/cli-kit/node/fs' | ||
| import {dirname, joinPath} from '@shopify/cli-kit/node/path' | ||
| import {Config} from '@oclif/core' | ||
| import {expect, test, vi} from 'vitest' | ||
| import {fileURLToPath} from 'node:url' | ||
|
|
||
| vi.mock('../../services/build.js') | ||
| vi.mock('@shopify/cli-kit/node/multiple-installation-warning') | ||
|
|
||
| test.each([false, true])('JSON stale-config recovery respects no-input: multiple replacements=%s', async (multiple) => { | ||
| await inTemporaryDirectory(async (directory) => { | ||
| const storage = new LocalStorage<localStorage.AppLocalStorageSchema>({cwd: joinPath(directory, 'cache')}) | ||
| await writeFile(joinPath(directory, 'package.json'), '{"name":"recovery-fixture"}') | ||
| await writeFile( | ||
| joinPath(directory, 'shopify.app.toml'), | ||
| `name = "Recovery fixture" | ||
| client_id = "public-fixture-id" | ||
| application_url = "https://example.com" | ||
| embedded = true | ||
| [auth] | ||
| redirect_urls = [] | ||
| [webhooks] | ||
| api_version = "2023-04" | ||
| `, | ||
| ) | ||
| if (multiple) { | ||
| await writeFile( | ||
| joinPath(directory, 'shopify.app.other.toml'), | ||
| await readFile(joinPath(directory, 'shopify.app.toml')), | ||
| ) | ||
| } | ||
| const readCache = localStorage.getCachedAppInfo | ||
| const writeCache = localStorage.setCachedAppInfo | ||
| const readCacheSpy = vi | ||
| .spyOn(localStorage, 'getCachedAppInfo') | ||
| .mockImplementation((directory) => readCache(directory, storage)) | ||
| const writeCacheSpy = vi | ||
| .spyOn(localStorage, 'setCachedAppInfo') | ||
| .mockImplementation((options) => writeCache(options, storage)) | ||
| vi.mocked(build).mockImplementation(async ({app}) => ({status: 'success', appName: app.name})) | ||
| vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', '1') | ||
| try { | ||
| localStorage.setCachedAppInfo({directory, configFile: 'shopify.app.deleted.toml'}) | ||
| const config = await Config.load({root: joinPath(dirname(fileURLToPath(import.meta.url)), '../../../..')}) | ||
| const argv = ['--path', directory, '--json', '--no-input'] | ||
| const command = new Build(argv, config) | ||
| await withCapturedStandardStreams(async ({stdout, stderr}) => { | ||
| const run = runWithCommandEventsForCommand(argv, () => command.run()) | ||
| if (multiple) { | ||
| await expect(run).rejects.toThrow('Failed to prompt') | ||
| expect(stdout()).toBe('') | ||
| expect(build).not.toHaveBeenCalled() | ||
| expect(localStorage.getCachedAppInfo(directory)?.configFile).toBe('shopify.app.deleted.toml') | ||
| return | ||
| } | ||
| await run | ||
| expect(JSON.parse(stdout())).toStrictEqual({status: 'success'}) | ||
| expect( | ||
| stderr() | ||
| .trim() | ||
| .split('\n') | ||
| .map((line) => JSON.parse(line)), | ||
| ).toContainEqual( | ||
| expect.objectContaining({ | ||
| type: 'diagnostic', | ||
| level: 'warning', | ||
| message: expect.stringContaining("Couldn't find shopify.app.deleted.toml"), | ||
| }), | ||
| ) | ||
| }) | ||
| if (!multiple) { | ||
| expect(vi.mocked(build).mock.calls[0]![0].app.configPath).toBe(joinPath(directory, 'shopify.app.toml')) | ||
| expect(localStorage.getCachedAppInfo(directory)?.configFile).toBe('shopify.app.toml') | ||
| } | ||
| } finally { | ||
| readCacheSpy.mockRestore() | ||
| writeCacheSpy.mockRestore() | ||
| vi.unstubAllEnvs() | ||
| } | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import Build from './build.js' | ||
| import build from '../../services/build.js' | ||
| import {localAppContext} from '../../services/app-context.js' | ||
| import {appBuildJsonOutputSchema} from '../../services/build/types.js' | ||
| import {testApp, testProject} from '../../models/app/app.test-data.js' | ||
| import {appFlags} from '../../flags.js' | ||
| import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' | ||
| import {withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output' | ||
| import {AbortSilentError} from '@shopify/cli-kit/node/error' | ||
| import {expect, test, vi} from 'vitest' | ||
| import {unstyled} from '@shopify/cli-kit/node/output' | ||
|
|
||
| vi.mock('../../services/build.js') | ||
| vi.mock('../../services/app-context.js') | ||
|
|
||
| function setup(directory: string) { | ||
| const app = testApp({name: 'Example app', directory, webs: []}) | ||
| vi.mocked(localAppContext).mockResolvedValue({app, project: testProject(), activeConfig: {} as never}) | ||
| vi.mocked(build).mockResolvedValue({status: 'success', appName: app.name}) | ||
| return app | ||
| } | ||
|
|
||
| test('declares JSON/schema/help and retains app and inherited flags', () => { | ||
| expect(Build.flags.json).toBeDefined() | ||
| expect(Build.flags.path).toBe(appFlags.path) | ||
| expect(Build.baseFlags).toHaveProperty('json-schema') | ||
| expect(Build.baseFlags['auth-alias']).toBeDefined() | ||
| expect(Build.jsonOutputSchema).toBe(appBuildJsonOutputSchema) | ||
| expect(Build.descriptionForHelp()).toContain('AppBuildResult') | ||
| }) | ||
|
|
||
| test.each([['--json'], ['--json', '--no-input'], ['--no-input']])( | ||
| 'JSON and no-input remain independent: %j', | ||
| async (...flags) => { | ||
| await inTemporaryDirectory(async (directory) => { | ||
| setup(directory) | ||
| await withCapturedStandardStreams(async ({stdout, stderr}) => { | ||
| await Build.run(['--path', directory, '--skip-dependencies-installation', ...flags], import.meta.url) | ||
| if (flags.includes('--json')) { | ||
| expect(JSON.parse(stdout())).toStrictEqual({status: 'success'}) | ||
| } else { | ||
| expect(stdout()).toBe('') | ||
| expect(unstyled(stderr())).toContain('Example app built!') | ||
| } | ||
| }) | ||
| expect(build).toHaveBeenCalledWith(expect.objectContaining({skipDependenciesInstallation: true})) | ||
| }) | ||
| }, | ||
| ) | ||
|
|
||
| test('silent build failure becomes one fatal JSON document, not an empty stdout', async () => { | ||
| await inTemporaryDirectory(async (directory) => { | ||
| setup(directory) | ||
| vi.mocked(build).mockRejectedValueOnce(new AbortSilentError()) | ||
| vi.stubEnv('SHOPIFY_FLAG_JSON', '1') | ||
| try { | ||
| await withCapturedStandardStreams(async ({stdout}) => { | ||
| await expect(Build.run(['--path', directory, '--json'], import.meta.url)).rejects.toThrow() | ||
| expect(JSON.parse(stdout())).toMatchObject({ | ||
| error: {type: 'abort', message: 'The app build did not complete. See the build diagnostics for details.'}, | ||
| }) | ||
| }) | ||
| } finally { | ||
| vi.unstubAllEnvs() | ||
| } | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import build from './build.js' | ||
| import buildWeb from './web.js' | ||
| import {installAppDependencies} from './dependencies.js' | ||
| import {installJavy} from './function/build.js' | ||
| import {appBuildJsonOutputSchema} from './build/types.js' | ||
| import {presentAppBuildResult} from './build/presenter.js' | ||
| import {testApp, testProject, testUIExtension} from '../models/app/app.test-data.js' | ||
| import {WebType} from '../models/app/app.js' | ||
| import {runWithCommandEventsForCommand} from '@shopify/cli-kit/node/command-events' | ||
| import {withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output' | ||
| import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' | ||
| import {joinPath} from '@shopify/cli-kit/node/path' | ||
| import {expect, test, vi} from 'vitest' | ||
|
|
||
| vi.mock('./web.js') | ||
| vi.mock('./dependencies.js') | ||
| vi.mock('./function/build.js') | ||
|
|
||
| test('build returns data and the real presenter reserves stdout for the JSON result', async () => { | ||
| await inTemporaryDirectory(async (directory) => { | ||
| const extension = await testUIExtension({directory: joinPath(directory, 'extension')}) | ||
| vi.spyOn(extension, 'build').mockImplementation(async ({stdout}) => { | ||
| stdout.write('extension built\n') | ||
| }) | ||
| vi.mocked(buildWeb).mockImplementation(async (_command, {stdout, stderr}) => { | ||
| stdout.write('web built\n') | ||
| stderr.write('build diagnostic\n') | ||
| }) | ||
| const app = testApp({ | ||
| name: 'Example app', | ||
| directory, | ||
| allExtensions: [extension], | ||
| webs: [{directory, configuration: {roles: [WebType.Backend], commands: {dev: '', build: 'external-build'}}}], | ||
| }) | ||
| await withCapturedStandardStreams(async ({stdout, stderr}) => { | ||
| await runWithCommandEventsForCommand(['--json'], async () => { | ||
| const result = await build({app, project: testProject(), skipDependenciesInstallation: true}) | ||
| expect(result).toStrictEqual({status: 'success', appName: 'Example app'}) | ||
| expect(stdout()).toBe('') | ||
| presentAppBuildResult(result, true) | ||
| }) | ||
| expect(JSON.parse(stdout())).toStrictEqual({status: 'success'}) | ||
| const messages = stderr() | ||
| .trim() | ||
| .split('\n') | ||
| .map((line) => JSON.parse(line).message) | ||
| expect(messages).toEqual( | ||
| expect.arrayContaining( | ||
| ['web built', 'build diagnostic', 'extension built'].map((message) => expect.stringContaining(message)), | ||
| ), | ||
| ) | ||
| expect(extension.build).toHaveBeenCalledOnce() | ||
| expect(installAppDependencies).not.toHaveBeenCalled() | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| test('empty apps still return a successful result', async () => { | ||
| const app = testApp({name: 'Empty app', webs: [], allExtensions: []}) | ||
| await expect(build({app, project: testProject(), skipDependenciesInstallation: true})).resolves.toStrictEqual({ | ||
| status: 'success', | ||
| appName: 'Empty app', | ||
| }) | ||
| expect(installJavy).toHaveBeenCalledWith(app) | ||
| }) | ||
|
|
||
| test.each([ | ||
| ['missing status', {}], | ||
| ['invalid status', {status: 'failed'}], | ||
| ['extra field', {status: 'success', appName: 'Example app'}], | ||
| ])('rejects %s', (_name, invalid) => { | ||
| expect(() => appBuildJsonOutputSchema.encode(invalid as never)).toThrow() | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import {appBuildJsonOutputSchema, type AppBuildResult} from './types.js' | ||
| import {outputResult} from '@shopify/cli-kit/node/output' | ||
| import {renderSuccess} from '@shopify/cli-kit/node/ui' | ||
|
|
||
| export function presentAppBuildResult(result: AppBuildResult, json: boolean): void { | ||
| if (json) { | ||
| outputResult(appBuildJsonOutputSchema.encode({status: result.status})) | ||
| } else { | ||
| renderSuccess({headline: [{userInput: result.appName}, 'built!']}) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import {defineJsonOutputSchema, type InferJsonOutputSchema} from '@shopify/cli-kit/node/json-output-schema' | ||
| import {zod} from '@shopify/cli-kit/node/schema' | ||
|
|
||
| export const appBuildJsonOutputSchema = defineJsonOutputSchema({ | ||
| name: 'AppBuildResult', | ||
| schema: zod.object({status: zod.literal('success')}).strict(), | ||
| }) | ||
|
|
||
| export type AppBuildResult = InferJsonOutputSchema<typeof appBuildJsonOutputSchema> & {appName: string} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.