diff --git a/.changeset/mighty-hotels-tickle.md b/.changeset/mighty-hotels-tickle.md new file mode 100644 index 00000000000..cabf99de88f --- /dev/null +++ b/.changeset/mighty-hotels-tickle.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Improve JSON Schema validation error messages for arrays. diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 206aa1d9471..e0fac11ab57 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -73,11 +73,21 @@ export interface ConfigurationError { code?: string } +function tomlObjectArrayHint(error: ConfigurationError): string | undefined { + if (!error.file.endsWith('.toml')) return undefined + if (error.message !== 'Expected object, received array') return undefined + + return 'Use a single TOML table instead of an array of tables. [table] defines one table; [[table]] defines an array of tables.' +} + export function formatConfigurationError(error: ConfigurationError): string { + const hint = tomlObjectArrayHint(error) + const message = hint ? `${error.message}. ${hint}` : error.message + if (error.path?.length) { - return `[${error.path.join('.')}]: ${error.message}` + return `[${error.path.join('.')}]: ${message}` } - return error.message + return message } type ConfigurationResult = {data: T; errors?: never} | {data?: never; errors: ConfigurationError[]} diff --git a/packages/app/src/cli/services/validate.test.ts b/packages/app/src/cli/services/validate.test.ts index f22f543286f..61d23135648 100644 --- a/packages/app/src/cli/services/validate.test.ts +++ b/packages/app/src/cli/services/validate.test.ts @@ -37,6 +37,28 @@ describe('formatConfigurationError', () => { '[access.admin]: Required', ) }) + + test('adds a TOML table hint for object array type mismatches', () => { + expect( + formatConfigurationError({ + file: 'shopify.app.toml', + path: ['events', '1', 'metrics'], + message: 'Expected object, received array', + }), + ).toBe( + '[events.1.metrics]: Expected object, received array. Use a single TOML table instead of an array of tables. [table] defines one table; [[table]] defines an array of tables.', + ) + }) + + test('does not add a TOML table hint for non-TOML files', () => { + expect( + formatConfigurationError({ + file: 'config.json', + path: ['events', '1', 'metrics'], + message: 'Expected object, received array', + }), + ).toBe('[events.1.metrics]: Expected object, received array') + }) }) describe('validateApp', () => { diff --git a/packages/cli-kit/src/public/node/json-schema.test.ts b/packages/cli-kit/src/public/node/json-schema.test.ts index 46583d30050..dd3a2eee4de 100644 --- a/packages/cli-kit/src/public/node/json-schema.test.ts +++ b/packages/cli-kit/src/public/node/json-schema.test.ts @@ -263,6 +263,87 @@ describe('jsonSchemaValidate', () => { expect(schemaParsed.errors, `Converting ${JSON.stringify(schemaParsed.rawErrors)}`).toEqual(zodErrors) }) + test('reports arrays distinctly when an object is expected', () => { + const subject = { + events: [ + {}, + { + metrics: [], + }, + ], + } + const contract = { + type: 'object', + properties: { + events: { + type: 'array', + items: { + type: 'object', + properties: { + metrics: {type: 'object'}, + }, + }, + }, + }, + } + + const schemaParsed = jsonSchemaValidate(subject, contract, 'strip') + + expect(schemaParsed.state).toBe('error') + expect(schemaParsed.errors).toEqual([ + { + path: ['events', '1', 'metrics'], + message: 'Expected object, received array', + }, + ]) + }) + + test('reports null distinctly when an object is expected', () => { + const subject = { + foo: null, + } + const contract = { + type: 'object', + properties: { + foo: {type: 'object'}, + }, + } + + const schemaParsed = jsonSchemaValidate(subject, contract, 'strip') + + expect(schemaParsed.state).toBe('error') + expect(schemaParsed.errors).toEqual([ + { + path: ['foo'], + message: 'Expected object, received null', + }, + ]) + }) + + test('reports top-level arrays distinctly when an object is expected', () => { + const schemaParsed = jsonSchemaValidate([], {type: 'object'}, 'strip') + + expect(schemaParsed.state).toBe('error') + expect(schemaParsed.errors).toEqual([ + { + path: [], + message: 'Expected object, received array', + }, + ]) + }) + + test('reports top-level null distinctly when an object is expected', () => { + const schemaParsed = jsonSchemaValidate(null as unknown as object, {type: 'object'}, 'strip') + + expect(schemaParsed.state).toBe('error') + expect(schemaParsed.errors).toEqual([ + { + path: [], + message: 'Expected object, received null', + }, + ]) + }) + test('ignores custom x-taplo directive', () => { const subject = { foo: 'bar', diff --git a/packages/cli-kit/src/public/node/json-schema.ts b/packages/cli-kit/src/public/node/json-schema.ts index e5cb6b2ec9f..ea8be968410 100644 --- a/packages/cli-kit/src/public/node/json-schema.ts +++ b/packages/cli-kit/src/public/node/json-schema.ts @@ -104,6 +104,22 @@ export function jsonSchemaValidate( } } +/** + * Get a more precise type name for JSON Schema error messages. + * + * @param value - The value to get the type of. + * @returns A string representing the type (e.g., 'array', 'null', 'object'). + */ +function getJsonSchemaValueType(value: unknown): string { + if (Array.isArray(value)) return 'array' + if (value === null) return 'null' + return typeof value +} + +function getJsonSchemaErrorValue(subject: object, path: string[]): unknown { + return path.length === 0 ? subject : getPathValue(subject, path) +} + /** * Converts errors from Ajv into a zod-like format. * @@ -128,8 +144,8 @@ function convertJsonSchemaErrors(rawErrors: AjvError[], subject: object, schema: const expectedType = Array.isArray(error.params.type) ? error.params.type.join(', ') : (error.params.type as string) - const actualType = getPathValue(subject, path.join('.')) - return {path, message: `Expected ${expectedType}, received ${typeof actualType}`} + const actualType = getJsonSchemaErrorValue(subject, path) + return {path, message: `Expected ${expectedType}, received ${getJsonSchemaValueType(actualType)}`} } if (error.keyword === 'anyOf' || error.keyword === 'oneOf') { @@ -138,7 +154,7 @@ function convertJsonSchemaErrors(rawErrors: AjvError[], subject: object, schema: if (error.params.allowedValues) { const allowedValues = error.params.allowedValues as string[] - const actualValue = getPathValue(subject, path.join('.')) + const actualValue = getJsonSchemaErrorValue(subject, path) return { path, message: `Invalid enum value. Expected ${allowedValues @@ -150,7 +166,7 @@ function convertJsonSchemaErrors(rawErrors: AjvError[], subject: object, schema: if (error.params.comparison) { const comparison = error.params.comparison as string const limit = error.params.limit - const actualValue = getPathValue(subject, path.join('.')) + const actualValue = getJsonSchemaErrorValue(subject, path) let comparisonText = comparison switch (comparison) {