Repository navigation
Keep extension flavor flag options in sync with supported values - #9029
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
An explicitly empty flavor bypasses validation and can select the downloaded repository root instead of the intended flavor subdirectory.
1 open issue
What changed in this PR
Moves extension flavor validation from a fixed command flag allowlist to template metadata, extending the Preact fix to other supported flavors without duplicating metadata.
Changes:
- Validates flavors against the selected template, or all fetched templates when none is selected.
- Adds coverage for flag and environment delegation, supported flavors, and invalid-flavor errors.
- Refreshes generated help and documentation and adds a patch changeset.
| File | Description |
|---|---|
packages/cli/README.md |
Updates generated flavor usage and help. |
packages/cli/oclif.manifest.json |
Removes fixed flavor options and updates help. |
packages/app/src/cli/services/generate.ts |
Validates flavors using template metadata. |
packages/app/src/cli/services/generate.test.ts |
Covers supported flavors and early rejection. |
packages/app/src/cli/commands/app/generate/extension.ts |
Delegates flavor validation to the generator. |
packages/app/src/cli/commands/app/generate/extension.test.ts |
Tests flag and environment delegation. |
docs-shopify.dev/generated/generated_docs_data_v2.json |
Refreshes generated flavor documentation. |
.changeset/template-based-extension-flavors.md |
Records the user-facing patch. |
Review effort: Balanced
| 'The Git URL to clone the function extensions templates from. Defaults to: https://github.com/Shopify/function-examples', | ||
| env: 'SHOPIFY_FLAG_CLONE_URL', | ||
| }), | ||
| flavor: Flags.string({ |
There was a problem hiding this comment.
Could we keep a finite list but derive it from ExtensionFlavorValue? e.g.
// services/generate/extension.ts
export const extensionFlavorValues = [
'vanilla-js', 'preact', 'react', 'typescript', 'typescript-react',
'rust', 'wasm', 'liquid', 'config-only',
] as const
export type ExtensionFlavorValue = (typeof extensionFlavorValues)[number]then Flags.custom<ExtensionFlavorValue>({options: [...extensionFlavorValues], ...})() here, like commands/app/security/blocking-flag.ts.
That fixes the drift from #9013 (the type already has liquid and config-only, and adding a flavor forces updating getSrcFileExtension), keeps the options in --help, and rejects typos and empty values when the flag is parsed. The generator would only see flavors the CLI knows how to scaffold, and your per-template check still handles "valid flavor, wrong template". The ''/future-flavor delegation tests would flip to rejection tests.
There was a problem hiding this comment.
Agreed—keeping the CLI-supported set separate from template compatibility is better. Implemented in e521b79: extensionFlavorValues now drives both ExtensionFlavorValue and the typed Flags.custom options, including liquid and config-only.
Empty values, unknown future flavors, and typos now fail during parsing; template-specific validation remains for known flavors used with the wrong template. Updated the rejection tests and regenerated help/manifest output. All 80 focused tests, lint, and app type-checking pass locally.
durga256
changed the title
| flavor: Flags.custom<ExtensionFlavorValue>({ | ||
| hidden: false, | ||
| description: | ||
| 'Choose a starting template for your extension, where applicable. Required if non interactive when the selected extension template supports multiple flavors.', | ||
| options: ['vanilla-js', 'preact', 'react', 'typescript', 'typescript-react', 'wasm', 'rust'], | ||
| 'Choose a starting template for your extension, where applicable. Supported flavors depend on the selected extension template. Required if non interactive when the selected extension template supports multiple flavors.', | ||
| options: [...extensionFlavorValues], | ||
| env: 'SHOPIFY_FLAG_FLAVOR', | ||
| }), | ||
| })(), |
There was a problem hiding this comment.
This is ok, but any reason why we can't use Flag.string here? extensionFlavorValues is still an array of strings no?
WHY are these changes introduced?
Follow-up to #9013 and the Slack discussion.
The
--flavorflag's options duplicateExtensionFlavorValueand can drift, rejecting CLI-supported flavors such asliquidandconfig-onlybefore the generator checks the selected template.WHAT is this pull request doing?
Define the CLI-supported flavors once, deriving both the TypeScript type and the flag's options from that list. This preserves the options in
--helpand rejects unknown or explicitly empty values during parsing, before loading the app.Keep template-metadata validation for known flavors used with an incompatible template, including a clear error when no fetched template supports a requested flavor. Add regression coverage and refresh the generated help and documentation.
How to manually test your changes?
Using a build or snapshot of this branch:
shopify app generate extension --help. Confirm the flavor options includepreact,liquid, andconfig-only, and explain that compatibility depends on the selected template.shopify app generate extension --template theme_app_extension --name liquid-flavor-check --flavor liquid --no-input. Confirm the extension is generated without a flavor prompt.shopify app generate extension --template admin_action --name invalid-flavor-check --flavor unknown --no-input. Confirm flag parsing rejects the unknown flavor and lists the CLI-supported options.shopify app generate extension --template admin_action --name incompatible-flavor-check --flavor rust --no-input. Confirm template validation rejects the known but incompatible flavor and lists the selected template's supported flavors.shopify app generate extension --template admin_action --name empty-flavor-check --flavor "" --no-input. Confirm flag parsing rejects the explicit empty value before loading the app or generating files.Checklist
patch) and added a changeset.