Repository navigation
Add typed JSON output to theme package - #8683
Conversation
9dce70c to
cbf4e74
Compare
0993a7c to
619ca97
Compare
cbf4e74 to
8290b55
Compare
20a9534 to
79ef9f7
Compare
8290b55 to
195f2b0
Compare
| vi.mock('@shopify/cli-kit/node/analytics', () => ({ | ||
| recordEvent: vi.fn(), | ||
| compileData: vi.fn().mockReturnValue({timings: {}, errors: {}, retries: {}, events: {}}), | ||
| })) |
There was a problem hiding this comment.
you don't need these special cases no? Just
vi.mock('@shopify/cli-kit/node/ui')
vi.mock('@shopify/cli-kit/node/analytics')
Should work fine
There was a problem hiding this comment.
Updated, thanks
isaacroldan
left a comment
There was a problem hiding this comment.
Minor comment on the tests
79ef9f7 to
a5c8e13
Compare
195f2b0 to
28d575f
Compare
a5c8e13 to
61b111d
Compare
28d575f to
29801b7
Compare
61b111d to
8a2dfe7
Compare
5f102b2 to
72d4123
Compare
00c153b to
5ad8644
Compare
901c0bc to
d5ad578
Compare
7542d09 to
a8cef4b
Compare
d5ad578 to
f62547a
Compare
a8cef4b to
ca2cebe
Compare
3034081 to
9aa6e95
Compare
124f20e to
f0044a6
Compare
9b568c8 to
86c3847
Compare
f0044a6 to
a393d78
Compare
86c3847 to
68b94cf
Compare
2f75003 to
f1fdf80
Compare
68b94cf to
0169d92
Compare
f1fdf80 to
eaeb644
Compare
0169d92 to
5c64f25
Compare
isaacroldan
left a comment
There was a problem hiding this comment.
Found one false success receipt.
🤖 Generated by the review-pr skill
| async command(flags: PackageFlags) { | ||
| await packageTheme(flags.path) | ||
| const result = await packageTheme(flags.path) | ||
| renderThemePackageResult(result, flags.json ? 'json' : 'text') |
There was a problem hiding this comment.
Can we make the archive writer reject output errors before emitting this receipt? With a valid settings_schema.json and an existing Dawn.zip directory, --json returns {path: ".../Dawn.zip"} and exits 0, but no ZIP was written. I checked this with the real writer and files. zip() currently resolves when the output stream closes even when opening it fails. This needs to use the fatal JSON error path.
🤖 Generated by the review-pr skill
eaeb644 to
f404ebe
Compare
6cba887 to
dc82947
Compare
be2a361 to
54d6696
Compare
dc82947 to
c06193e
Compare
83aa53c to
b6a9981
Compare
c06193e to
771e44a
Compare
|
| Changeset | Package |
|---|---|
theme-check-json-contract.md |
'@shopify/cli': major |
theme-duplicate-json-contract.md |
'@shopify/cli': major |
theme-info-json-contract.md |
'@shopify/cli': major |
theme-list-json-contract.md |
'@shopify/cli': major |
theme-open-json-contract.md |
'@shopify/cli': major |
theme-preview-json-contract.md |
'@shopify/cli': major |
theme-profile-environments.md |
'@shopify/cli': major |
theme-push-json-contract.md |
"@shopify/cli": major |
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/base-command.d.ts@@ -20,6 +20,7 @@ declare abstract class BaseCommand extends Command {
static descriptionWithoutMarkdown(): string | undefined;
static analyticsNameOverride(): string | undefined;
static analyticsStopCommand(): string | undefined;
+ private jsonOutputFromEnvironment;
catch(error: Error & {
skipOclifErrorHandling: boolean;
}): Promise<void>;
@@ -35,6 +36,7 @@ declare abstract class BaseCommand extends Command {
argv: string[];
}>;
protected environmentsFilename(): string | undefined;
+ protected validateNonTTYFlags(flags: FlagOutput): void;
protected failMissingNonTTYFlags(flags: FlagOutput, requiredFlags: string[]): void;
private failMissingNonTTYFlagRequirements;
private applicableNonTTYFlagRequirements;
packages/cli-kit/dist/public/node/themes/theme-manager.d.ts@@ -11,5 +11,12 @@ export declare abstract class ThemeManager {
findOrCreate(name?: string, role?: Role): Promise<Theme>;
fetch(name?: string, role?: Role): Promise<Theme | undefined>;
generateThemeName(context: string): string;
- create(themeRole?: Role, themeName?: string): Promise<Theme>;
+ create(themeRole?: Role, themeName?: string): Promise<{
+ createdAtRuntime: boolean;
+ id: number;
+ name: string;
+ processing: boolean;
+ role: string;
+ src?: string;
+ }>;
}
\ No newline at end of file
|
WHY are these changes introduced?
Packaging needs a machine-readable artifact receipt.
WHAT is this pull request doing?
Return a strict JSON artifact result with a validated absolute path to the ZIP actually written, while preserving the text presenter.
The public output follows
docs/cli/json-output.md; explicitly named environments use one{environments: [...]}object.How to manually test your changes?
shopify theme package --path . --json shopify theme package --json-schemaChecklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add