-
Notifications
You must be signed in to change notification settings - Fork 234
Preserve structured app validation issues internally #7066
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
base: main
Are you sure you want to change the base?
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 | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,13 @@ | ||||||||||||||||||
| import type {OutputMessage} from '@shopify/cli-kit/node/output' | ||||||||||||||||||
|
|
||||||||||||||||||
| interface UnionErrorIssue { | ||||||||||||||||||
| path?: (string | number)[] | ||||||||||||||||||
| message: string | ||||||||||||||||||
| code?: string | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| interface UnionError { | ||||||||||||||||||
| issues?: {path?: (string | number)[]; message: string}[] | ||||||||||||||||||
| issues?: UnionErrorIssue[] | ||||||||||||||||||
| name: string | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -10,6 +18,20 @@ interface ExtendedZodIssue { | |||||||||||||||||
| unionErrors?: UnionError[] | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| export interface AppValidationIssue { | ||||||||||||||||||
| filePath: string | ||||||||||||||||||
| path: (string | number)[] | ||||||||||||||||||
| pathString: string | ||||||||||||||||||
| message: string | ||||||||||||||||||
| code?: string | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| export interface AppValidationFileIssues { | ||||||||||||||||||
| filePath: string | ||||||||||||||||||
| message: OutputMessage | ||||||||||||||||||
| issues: AppValidationIssue[] | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Finds the best matching variant from a union error by scoring each variant | ||||||||||||||||||
| * based on how close it is to the user's likely intent. | ||||||||||||||||||
|
|
@@ -56,13 +78,66 @@ function findBestMatchingVariant(unionErrors: UnionError[]): UnionError | null { | |||||||||||||||||
| return bestVariant | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| function getIssuePath(path?: (string | number)[]) { | ||||||||||||||||||
| return path ?? [] | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| function getIssuePathString(path?: (string | number)[]) { | ||||||||||||||||||
| const resolvedPath = getIssuePath(path) | ||||||||||||||||||
| return resolvedPath.length > 0 ? resolvedPath.map(String).join('.') : 'root' | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Formats an issue into a human-readable error line | ||||||||||||||||||
| */ | ||||||||||||||||||
| function formatErrorLine(issue: {path?: (string | number)[]; message?: string}, indent = '') { | ||||||||||||||||||
| const path = issue.path && issue.path.length > 0 ? issue.path.map(String).join('.') : 'root' | ||||||||||||||||||
| const pathString = getIssuePathString(issue.path) | ||||||||||||||||||
| const message = issue.message ?? 'Unknown error' | ||||||||||||||||||
| return `${indent}• [${path}]: ${message}\n` | ||||||||||||||||||
| return `${indent}• [${pathString}]: ${message}\n` | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| function toStructuredIssue(filePath: string, issue: Pick<ExtendedZodIssue, 'path' | 'message' | 'code'>) { | ||||||||||||||||||
| return { | ||||||||||||||||||
| filePath, | ||||||||||||||||||
| path: getIssuePath(issue.path), | ||||||||||||||||||
| pathString: getIssuePathString(issue.path), | ||||||||||||||||||
| message: issue.message ?? 'Unknown error', | ||||||||||||||||||
| code: issue.code, | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| export function parseStructuredErrors(issues: ExtendedZodIssue[], filePath: string): AppValidationIssue[] { | ||||||||||||||||||
| return issues.flatMap((issue) => { | ||||||||||||||||||
| if (issue.code === 'invalid_union' && issue.unionErrors) { | ||||||||||||||||||
| // Intentionally mirror the current human-readable union selection heuristic | ||||||||||||||||||
| // so structured/internal issues stay aligned with existing CLI behavior. | ||||||||||||||||||
| // If we change this heuristic later, text and structured output should move together. | ||||||||||||||||||
| const bestVariant = findBestMatchingVariant(issue.unionErrors) | ||||||||||||||||||
|
|
||||||||||||||||||
| if (bestVariant?.issues?.length) { | ||||||||||||||||||
| return bestVariant.issues.map((nestedIssue) => toStructuredIssue(filePath, nestedIssue)) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const fallbackIssues = issue.unionErrors.flatMap((unionError) => unionError.issues ?? []) | ||||||||||||||||||
| if (fallbackIssues.length > 0) { | ||||||||||||||||||
| // Preserve any concrete nested issues we were able to recover before | ||||||||||||||||||
| // falling back to a synthetic root issue. This structured path is still | ||||||||||||||||||
| // internal, and retaining leaf issues is more actionable than erasing | ||||||||||||||||||
| // them behind a generic union failure. | ||||||||||||||||||
| return fallbackIssues.map((nestedIssue) => toStructuredIssue(filePath, nestedIssue)) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
|
Comment on lines
+121
to
+129
|
||||||||||||||||||
| const fallbackIssues = issue.unionErrors.flatMap((unionError) => unionError.issues ?? []) | |
| if (fallbackIssues.length > 0) { | |
| // Preserve any concrete nested issues we were able to recover before | |
| // falling back to a synthetic root issue. This structured path is still | |
| // internal, and retaining leaf issues is more actionable than erasing | |
| // them behind a generic union failure. | |
| return fallbackIssues.map((nestedIssue) => toStructuredIssue(filePath, nestedIssue)) | |
| } |
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.
AppValidationFileIssues.messageis typed asOutputMessage(which can be aTokenizedString). If this structure is intended to be JSON-serialized (as the name suggests),TokenizedStringwill serialize as an object ({"value": ...}) rather than the rendered string. Consider changingmessageto a plainstring(produced viastringifyMessage) or renaming this type/method to avoid implying JSON-safe output.