Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mighty-hotels-tickle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/cli-kit': patch
---

Improve JSON Schema validation error messages for arrays.
14 changes: 12 additions & 2 deletions packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = {data: T; errors?: never} | {data?: never; errors: ConfigurationError[]}
Expand Down
22 changes: 22 additions & 0 deletions packages/app/src/cli/services/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
81 changes: 81 additions & 0 deletions packages/cli-kit/src/public/node/json-schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
24 changes: 20 additions & 4 deletions packages/cli-kit/src/public/node/json-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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') {
Expand All @@ -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
Expand All @@ -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) {
Expand Down
Loading