Add typed JSON output to Hydrogen project and account commands - #4101
gonzaloriestra wants to merge 1 commit into
Conversation
cb5cc4d to
d8ae8a8
Compare
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
d8ae8a8 to
8ee3647
Compare
8ee3647 to
bb0c78c
Compare
bb0c78c to
7c1c770
Compare
fredericoo
left a comment
There was a problem hiding this comment.
Went through this one. It's a clean follow-on from the rest of the stack: same jsonOutputSchema / writeJsonResult / descriptionForHelp pattern as the env and route PRs, results carry no tokens (login just picks shop/shopName/email), and the human-readable output looks unchanged for all six commands. Moving renderProjectReady and the remote-template info into presentTemplateResult is a nice split.
Prompts in --json mode look fine too. When stdout is piped, cli-kit's throwInNonTTY rejects them, so login/link/init fail fast with a JSON error instead of hanging.
Two small comments, neither blocking. Overall LGTM
| }); | ||
|
|
||
| if (!linkedStore) return; | ||
| const result = {shop: config.shop!, storefront: linkedStore ?? null}; |
There was a problem hiding this comment.
non-blocking: list takes shop from session.storeFqdn, but this takes it from config.shop!. Let's use the session here as well. It's the same authenticated shop, the two commands then read it from the same place, and we get rid of the !:
| const result = {shop: config.shop!, storefront: linkedStore ?? null}; | |
| const result = {shop: session.storeFqdn, storefront: linkedStore ?? null}; |
| if (isJsonOutput()) { | ||
| await errorHandler( | ||
| new AbortError( | ||
| 'Failed to initialize project: ' + (error?.message ?? ''), | ||
| error?.tryMessage ?? error?.stack, | ||
| ), | ||
| ); | ||
| await flushStdout(); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
non-blocking: the AbortError is built twice now, once for this JSON branch and once for renderFatalError below, and the two copies could drift. Let's build it once above the if and pass it to both.
Also, this JSON fatal path isn't tested. project-json.test.ts covers presentTemplateResult, but nothing checks that an init failure in --json mode emits the error event and exits 1. One test with process.exit spied would cover it. Note that this branch also skips the SHOPIFY_UNIT_TEST guard below, so a test that hits it without spying on process.exit would kill the runner.
WHY are these changes introduced?
Project creation, storefront linking and account commands need finite results for automation.
WHAT is this pull request doing?
Add domain schemas,
--jsonand schema help toinit,link,list,login,logoutandunlink. Separate onboarding execution from final presentation, report partial setup failures, and preserve the shared fatal-error path during asynchronous cleanup. Results omit authentication tokens.Without
--json, the existing terminal presentation remains. Example JSON result:{"unlinked":false,"storefront":null}HOW to test your changes?
pnpm --filter @shopify/cli-hydrogen test src/commands/hydrogen/project-json.test.ts src/lib/onboarding/local.test.tsBuild and typecheck were checked at each layer. The full stack passed 498 tests (6 skipped), lint, and schema discovery for all 22 finite commands. Command-line smoke checks covered a successful unlink result, fatal project errors, watch-mode rejection and schema environment flags. Live authenticated Shopify operations have not been exercised.
Stack
Draft 7 of 9. Previous: #4100. Next: #4102. One changeset is included in #4103.