diff --git a/.changeset/valid-range-default.md b/.changeset/valid-range-default.md new file mode 100644 index 000000000..34e6366c0 --- /dev/null +++ b/.changeset/valid-range-default.md @@ -0,0 +1,10 @@ +--- +'@shopify/theme-check-common': minor +--- + +Add two checks for `range` setting structural validity: + +- `ValidRangeDefault` — errors when a `range` setting's `default` value is outside `[min, max]` or not aligned to the `step` grid. Catches schemas like `{ "type": "range", "min": 0, "max": 160, "step": 8, "default": 60 }` that the storefront rejects with `Invalid schema: setting with id="…" default must be a step in the range`. +- `ValidRangeStepCount` — errors when a range setting has more than 101 discrete steps. Catches schemas like `{ "type": "range", "min": 0, "max": 200, "step": 1 }` that the storefront rejects with `Invalid schema: setting with id="…" step invalid. Range settings must have at most 101 steps`. + +Both checks validate setting defaults, preset setting values, section `default.settings` values, and `config/settings_schema.json`. Both are runtime failures that the CLI happily uploads but `shopify theme dev` and the theme editor reject as hard schema errors. diff --git a/packages/theme-check-common/src/checks/index.ts b/packages/theme-check-common/src/checks/index.ts index be0370c41..8d6068832 100644 --- a/packages/theme-check-common/src/checks/index.ts +++ b/packages/theme-check-common/src/checks/index.ts @@ -59,6 +59,8 @@ import { ValidJSON } from './valid-json'; import { ValidDocParamTypes } from './valid-doc-param-types'; import { ValidLocalBlocks } from './valid-local-blocks'; import { ValidRenderSnippetArgumentTypes } from './valid-render-snippet-argument-types'; +import { ValidRangeDefault, ValidRangeDefaultSettingsSchema } from './valid-range-default'; +import { ValidRangeStepCount, ValidRangeStepCountSettingsSchema } from './valid-range-step-count'; import { ValidSchema } from './valid-schema'; import { ValidSchemaName } from './valid-schema-name'; import { ValidSchemaTranslations } from './valid-schema-translations'; @@ -130,6 +132,10 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [ ValidJSON, ValidDocParamTypes, ValidLocalBlocks, + ValidRangeDefault, + ValidRangeDefaultSettingsSchema, + ValidRangeStepCount, + ValidRangeStepCountSettingsSchema, ValidRenderSnippetArgumentTypes, ValidSchema, ValidSettingsKey, diff --git a/packages/theme-check-common/src/checks/valid-range-default/index.spec.ts b/packages/theme-check-common/src/checks/valid-range-default/index.spec.ts new file mode 100644 index 000000000..8c799896c --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-range-default/index.spec.ts @@ -0,0 +1,355 @@ +import { describe, expect, it } from 'vitest'; +import { check, runJSONCheck } from '../../test'; +import { ValidRangeDefault, ValidRangeDefaultSettingsSchema } from './index'; + +function toLiquidFile(content: unknown) { + return ` + {% schema %} + ${JSON.stringify(content)} + {% endschema %} + `; +} + +describe('Module: ValidRangeDefault (Liquid schema)', () => { + describe('setting defaults', () => { + it('does not report when the default is aligned to the step grid', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'padding_top', + min: 0, + max: 160, + step: 8, + default: 64, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(0); + }); + + it('reports when the default is off the step grid', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'padding_top', + min: 0, + max: 160, + step: 8, + default: 60, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/padding_top/); + expect(offenses[0].message).toMatch(/60/); + expect(offenses[0].message).toMatch(/step 8/); + expect(offenses[0].message).toMatch(/try 64/); + }); + + it('reports when the default is below min', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'gap', + min: 10, + max: 60, + step: 5, + default: 0, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/outside the range \[10, 60\]/); + }); + + it('reports when the default is above max', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'gap', + min: 0, + max: 100, + step: 10, + default: 200, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/outside the range \[0, 100\]/); + }); + + it('handles fractional steps without floating-point false positives', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'rating', + min: 1, + max: 5, + step: 0.1, + default: 4.7, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(0); + }); + + it('reports an off-step fractional default', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'gap', + min: 0, + max: 10, + step: 0.5, + default: 2.3, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/gap/); + expect(offenses[0].message).toMatch(/step 0.5/); + }); + + it('does not report when the range setting has no default', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'padding_top', + min: 0, + max: 160, + step: 8, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(0); + }); + + it('skips settings missing min/max/step (let JSON schema validation handle it)', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'padding_top', + default: 60, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(0); + }); + + it('ignores non-range settings', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { type: 'text', id: 'heading', default: 'Hello' }, + { type: 'number', id: 'count', default: 7 }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(0); + }); + + it('reports multiple invalid range defaults in one schema', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { + type: 'range', + id: 'padding_top', + min: 0, + max: 160, + step: 8, + default: 60, + }, + { + type: 'range', + id: 'padding_bottom', + min: 0, + max: 160, + step: 8, + default: 60, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(2); + }); + }); + + describe('preset settings', () => { + it('does not report when a preset setting is on the step grid', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'padding_top', min: 0, max: 160, step: 8 }], + presets: [{ name: 'Default', settings: { padding_top: 48 } }], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(0); + }); + + it('reports when a preset setting is off the step grid', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'padding_top', min: 0, max: 160, step: 8 }], + presets: [{ name: 'Default', settings: { padding_top: 60 } }], + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/padding_top/); + expect(offenses[0].message).toMatch(/60/); + }); + + it('reports invalid values in section default.settings', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'padding_top', min: 0, max: 160, step: 8 }], + default: { settings: { padding_top: 60 } }, + }), + }; + + const offenses = await check(theme, [ValidRangeDefault]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/padding_top/); + }); + }); +}); + +describe('Module: ValidRangeDefaultSettingsSchema (config/settings_schema.json)', () => { + it('reports an off-step default in config/settings_schema.json', async () => { + const source = JSON.stringify([ + { + name: 'Layout', + settings: [ + { + type: 'range', + id: 'logo_width', + min: 50, + max: 200, + step: 4, + default: 75, + }, + ], + }, + ]); + + const offenses = await runJSONCheck( + ValidRangeDefaultSettingsSchema, + source, + 'config/settings_schema.json', + ); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/logo_width/); + expect(offenses[0].message).toMatch(/75/); + expect(offenses[0].message).toMatch(/step 4/); + }); + + it('does not report a valid default', async () => { + const source = JSON.stringify([ + { + name: 'Layout', + settings: [ + { + type: 'range', + id: 'logo_width', + min: 50, + max: 200, + step: 4, + default: 74, + }, + ], + }, + ]); + + const offenses = await runJSONCheck( + ValidRangeDefaultSettingsSchema, + source, + 'config/settings_schema.json', + ); + expect(offenses).toHaveLength(0); + }); + + it('does not run on files other than settings_schema.json', async () => { + const source = JSON.stringify([ + { + name: 'Layout', + settings: [ + { + type: 'range', + id: 'logo_width', + min: 50, + max: 200, + step: 4, + default: 75, + }, + ], + }, + ]); + + const offenses = await runJSONCheck( + ValidRangeDefaultSettingsSchema, + source, + 'config/other.json', + ); + expect(offenses).toHaveLength(0); + }); +}); diff --git a/packages/theme-check-common/src/checks/valid-range-default/index.ts b/packages/theme-check-common/src/checks/valid-range-default/index.ts new file mode 100644 index 000000000..1d53244c2 --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-range-default/index.ts @@ -0,0 +1,260 @@ +import { + JSONCheckDefinition, + JSONNode, + LiquidCheckDefinition, + Setting, + Severity, + SourceCodeType, + isArrayNode, + isLiteralNode, + isObjectNode, +} from '../../types'; +import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; +import { getSchema, isSectionSchema } from '../../to-schema'; + +// Note: like ValidVisibleIf and ValidSelectDefault, this exports two checks: +// one for Liquid files ({% schema %} in sections/blocks) and one for +// 'config/settings_schema.json'. Same logic, two surface points. + +// Storefronts reject schemas where a `range` setting's `default` falls outside +// [min, max] or off the (min + n*step) grid. The CLI uploads the file fine, +// but `shopify theme dev` and the theme editor surface this as a hard +// "Invalid schema" error at render time. This check catches it statically. + +const EPSILON = 1e-9; + +const meta = { + code: 'ValidRangeDefault', + name: 'Validate default values and preset values for range settings', + docs: { + description: + "Warns when a range setting's default value (or a preset's setting value) is outside [min, max] or not aligned to the step grid.", + recommended: true, + url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/valid-range-default', + }, + severity: Severity.ERROR, + schema: {}, + targets: [], +}; + +type RangeSetting = { + id: string; + type: 'range'; + min: number; + max: number; + step: number; + default?: unknown; +}; + +function isFiniteNumber(value: unknown): value is number { + return typeof value === 'number' && Number.isFinite(value); +} + +function getRangeSettings(settings: readonly Setting.Any[] | undefined): RangeSetting[] { + if (!settings) return []; + const result: RangeSetting[] = []; + for (const setting of settings) { + if (!setting || typeof setting !== 'object') continue; + if (!('type' in setting) || setting.type !== 'range') continue; + if (!('id' in setting) || typeof setting.id !== 'string') continue; + const { min, max, step } = setting as { min?: unknown; max?: unknown; step?: unknown }; + if (!isFiniteNumber(min) || !isFiniteNumber(max) || !isFiniteNumber(step)) continue; + if (step <= 0) continue; + result.push(setting as unknown as RangeSetting); + } + return result; +} + +function isOnStepGrid(setting: RangeSetting, value: number): boolean { + const ratio = (value - setting.min) / setting.step; + return Math.abs(ratio - Math.round(ratio)) < EPSILON; +} + +function nearestValidValue(setting: RangeSetting, value: number): number { + const clamped = Math.min(setting.max, Math.max(setting.min, value)); + const steps = Math.round((clamped - setting.min) / setting.step); + const candidate = setting.min + steps * setting.step; + return Math.min(setting.max, Math.max(setting.min, candidate)); +} + +function rangeProblem(setting: RangeSetting, value: number): string | undefined { + if (value < setting.min || value > setting.max) { + return `is outside the range [${setting.min}, ${setting.max}]`; + } + if (!isOnStepGrid(setting, value)) { + const suggestion = nearestValidValue(setting, value); + return `is not aligned to step ${setting.step} in range [${setting.min}, ${setting.max}] — try ${suggestion}`; + } + return undefined; +} + +function invalidDefaultMessage(setting: RangeSetting, value: number): string { + return `Default value ${value} for range setting "${setting.id}" ${rangeProblem(setting, value)}.`; +} + +function invalidPresetMessage(setting: RangeSetting, value: number): string { + return `Value ${value} for range setting "${setting.id}" ${rangeProblem(setting, value)}.`; +} + +export const ValidRangeDefault: LiquidCheckDefinition = { + meta: { ...meta, type: SourceCodeType.LiquidHtml }, + + create(context) { + return { + async LiquidRawTag(node) { + if (node.name !== 'schema' || node.body.kind !== 'json') return; + + const schema = await getSchema(context); + const { validSchema, ast } = schema ?? {}; + if (!validSchema || validSchema instanceof Error) return; + if (!ast || ast instanceof Error) return; + + const offset = node.blockStartPosition.end; + const rangeSettings = getRangeSettings(validSchema.settings); + if (rangeSettings.length === 0) return; + + const settingsById = new Map(rangeSettings.map((s) => [s.id, s])); + + // 1. Validate setting defaults + for (let i = 0; i < (validSchema.settings?.length ?? 0); i++) { + const setting = validSchema.settings![i]; + if (!setting || !('id' in setting)) continue; + const range = settingsById.get(setting.id as string); + if (!range) continue; + if (!('default' in range) || range.default === undefined) continue; + if (!isFiniteNumber(range.default)) continue; + if (!rangeProblem(range, range.default)) continue; + + const defaultNode = nodeAtPath(ast, ['settings', i, 'default']); + if (defaultNode) { + reportAtNode(context, offset, defaultNode, invalidDefaultMessage(range, range.default)); + } + } + + // 2. Validate presets[].settings values + if (Array.isArray(validSchema.presets)) { + for (let i = 0; i < validSchema.presets.length; i++) { + validatePresetSettings( + context, + offset, + ast, + ['presets', String(i), 'settings'], + settingsById, + ); + } + } + + // 3. Validate section default.settings values + if (isSectionSchema(schema) && 'default' in validSchema && validSchema.default?.settings) { + validatePresetSettings(context, offset, ast, ['default', 'settings'], settingsById); + } + }, + }; + }, +}; + +function validatePresetSettings( + context: Parameters[0], + offset: number, + ast: JSONNode, + settingsPath: string[], + settingsById: Map, +) { + const settingsNode = nodeAtPath(ast, settingsPath); + if (!settingsNode || !isObjectNode(settingsNode)) return; + + for (const property of settingsNode.children) { + const key = property.key.value; + if (typeof key !== 'string') continue; + const range = settingsById.get(key); + if (!range) continue; + + const valueNode = property.value; + if (!isLiteralNode(valueNode)) continue; + if (!isFiniteNumber(valueNode.value)) continue; + if (!rangeProblem(range, valueNode.value)) continue; + + reportAtNode(context, offset, valueNode, invalidPresetMessage(range, valueNode.value)); + } +} + +function reportAtNode( + context: Parameters[0], + offset: number, + astNode: JSONNode, + message: string, +) { + context.report({ + message, + startIndex: offset + getLocStart(astNode), + endIndex: offset + getLocEnd(astNode), + }); +} + +export const ValidRangeDefaultSettingsSchema: JSONCheckDefinition = { + meta: { ...meta, type: SourceCodeType.JSON }, + + create(context) { + const relativePath = context.toRelativePath(context.file.uri); + if (relativePath !== 'config/settings_schema.json') return {}; + + return { + async Property(node) { + if (node.key.value !== 'settings' || !isArrayNode(node.value)) return; + + for (const settingNode of node.value.children) { + if (!isObjectNode(settingNode)) continue; + + const typeProp = settingNode.children.find((p) => p.key.value === 'type'); + if (!typeProp || !isLiteralNode(typeProp.value) || typeProp.value.value !== 'range') { + continue; + } + + const idProp = settingNode.children.find((p) => p.key.value === 'id'); + const idValue = + idProp && isLiteralNode(idProp.value) && typeof idProp.value.value === 'string' + ? idProp.value.value + : 'unknown'; + + const minProp = settingNode.children.find((p) => p.key.value === 'min'); + const maxProp = settingNode.children.find((p) => p.key.value === 'max'); + const stepProp = settingNode.children.find((p) => p.key.value === 'step'); + const defaultProp = settingNode.children.find((p) => p.key.value === 'default'); + if (!minProp || !maxProp || !stepProp || !defaultProp) continue; + if ( + !isLiteralNode(minProp.value) || + !isLiteralNode(maxProp.value) || + !isLiteralNode(stepProp.value) || + !isLiteralNode(defaultProp.value) + ) { + continue; + } + + const min = minProp.value.value; + const max = maxProp.value.value; + const step = stepProp.value.value; + const defaultValue = defaultProp.value.value; + if ( + !isFiniteNumber(min) || + !isFiniteNumber(max) || + !isFiniteNumber(step) || + !isFiniteNumber(defaultValue) + ) { + continue; + } + if (step <= 0) continue; + + const range: RangeSetting = { id: idValue, type: 'range', min, max, step }; + const problem = rangeProblem(range, defaultValue); + if (!problem) continue; + + context.report({ + message: `Default value ${defaultValue} for range setting "${idValue}" ${problem}.`, + startIndex: getLocStart(defaultProp.value), + endIndex: getLocEnd(defaultProp.value), + }); + } + }, + }; + }, +}; diff --git a/packages/theme-check-common/src/checks/valid-range-step-count/index.spec.ts b/packages/theme-check-common/src/checks/valid-range-step-count/index.spec.ts new file mode 100644 index 000000000..827d42efe --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-range-step-count/index.spec.ts @@ -0,0 +1,185 @@ +import { describe, expect, it } from 'vitest'; +import { check, runJSONCheck } from '../../test'; +import { ValidRangeStepCount, ValidRangeStepCountSettingsSchema } from './index'; + +function toLiquidFile(content: unknown) { + return ` + {% schema %} + ${JSON.stringify(content)} + {% endschema %} + `; +} + +describe('Module: ValidRangeStepCount (Liquid schema)', () => { + it('does not report when the step count is at the 101 limit', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'width', min: 0, max: 100, step: 1 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(0); + }); + + it('reports when the step count exceeds 101', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'width', min: 0, max: 200, step: 1 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/width/); + expect(offenses[0].message).toMatch(/201 steps/); + expect(offenses[0].message).toMatch(/at most 101/); + }); + + it('reports a step count that just barely exceeds the limit', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'width', min: 0, max: 101, step: 1 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/102 steps/); + }); + + it('does not report fractional ranges within the limit', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'rating', min: 1, max: 5, step: 0.1 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(0); + }); + + it('reports a fractional step that creates more than 101 values', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'value', min: 0, max: 20, step: 0.1 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/value/); + expect(offenses[0].message).toMatch(/201 steps/); + }); + + it('suggests a step size that fits within the limit', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'width', min: 0, max: 200, step: 1 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/step ≥ 2/); + }); + + it('skips settings missing min/max/step', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'range', id: 'width', default: 50 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(0); + }); + + it('ignores non-range settings', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [{ type: 'number', id: 'count', default: 7 }], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(0); + }); + + it('reports each over-budget range setting independently', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + name: 'Example', + settings: [ + { type: 'range', id: 'a', min: 0, max: 500, step: 1 }, + { type: 'range', id: 'b', min: 0, max: 50, step: 1 }, + { type: 'range', id: 'c', min: 0, max: 1000, step: 1 }, + ], + }), + }; + + const offenses = await check(theme, [ValidRangeStepCount]); + expect(offenses).toHaveLength(2); + expect(offenses.map((o) => o.message).join('\n')).toMatch(/"a"/); + expect(offenses.map((o) => o.message).join('\n')).toMatch(/"c"/); + }); +}); + +describe('Module: ValidRangeStepCountSettingsSchema (config/settings_schema.json)', () => { + it('reports an over-budget range setting in config/settings_schema.json', async () => { + const source = JSON.stringify([ + { + name: 'Layout', + settings: [{ type: 'range', id: 'logo_width', min: 0, max: 200, step: 1 }], + }, + ]); + + const offenses = await runJSONCheck( + ValidRangeStepCountSettingsSchema, + source, + 'config/settings_schema.json', + ); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toMatch(/logo_width/); + }); + + it('does not report a within-budget range setting', async () => { + const source = JSON.stringify([ + { + name: 'Layout', + settings: [{ type: 'range', id: 'logo_width', min: 50, max: 200, step: 2 }], + }, + ]); + + const offenses = await runJSONCheck( + ValidRangeStepCountSettingsSchema, + source, + 'config/settings_schema.json', + ); + expect(offenses).toHaveLength(0); + }); + + it('does not run on files other than settings_schema.json', async () => { + const source = JSON.stringify([ + { + name: 'Layout', + settings: [{ type: 'range', id: 'logo_width', min: 0, max: 200, step: 1 }], + }, + ]); + + const offenses = await runJSONCheck( + ValidRangeStepCountSettingsSchema, + source, + 'config/other.json', + ); + expect(offenses).toHaveLength(0); + }); +}); diff --git a/packages/theme-check-common/src/checks/valid-range-step-count/index.ts b/packages/theme-check-common/src/checks/valid-range-step-count/index.ts new file mode 100644 index 000000000..b9b8f212c --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-range-step-count/index.ts @@ -0,0 +1,176 @@ +import { + JSONCheckDefinition, + JSONNode, + LiquidCheckDefinition, + Setting, + Severity, + SourceCodeType, + isArrayNode, + isLiteralNode, + isObjectNode, +} from '../../types'; +import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; +import { getSchema } from '../../to-schema'; + +// Shopify rejects range settings whose step count exceeds 101. The CLI +// uploads the file fine, but `shopify theme dev` and the theme editor +// surface this as: +// Invalid schema: setting with id="…" step invalid. +// Range settings must have at most 101 steps +// which is a hard 500 on the entire local theme server. +// +// Step count is (max - min) / step + 1. + +const MAX_STEPS = 101; +const EPSILON = 1e-9; + +const meta = { + code: 'ValidRangeStepCount', + name: 'Validate that range settings have at most 101 steps', + docs: { + description: + 'Warns when a range setting has more than 101 discrete steps, which the storefront rejects with "Range settings must have at most 101 steps".', + recommended: true, + url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/valid-range-step-count', + }, + severity: Severity.ERROR, + schema: {}, + targets: [], +}; + +function isFiniteNumber(value: unknown): value is number { + return typeof value === 'number' && Number.isFinite(value); +} + +function rawStepCount(min: number, max: number, step: number): number { + return (max - min) / step + 1; +} + +function exceedsMaxSteps(min: number, max: number, step: number): boolean { + // EPSILON absorbs float jitter so e.g. step 0.1 across (1, 5) doesn't + // tip from 41 to 41.00000000000001 and trip the > 101 comparison. + return rawStepCount(min, max, step) > MAX_STEPS + EPSILON; +} + +function tooManyStepsMessage(id: string, min: number, max: number, step: number): string { + const count = Math.round(rawStepCount(min, max, step)); + // Smallest step that keeps (max - min) / step + 1 <= 101. + const suggestion = (max - min) / (MAX_STEPS - 1); + return `Range setting "${id}" has ${count} steps (min ${min}, max ${max}, step ${step}). Shopify allows at most ${MAX_STEPS} steps — use step ≥ ${suggestion}.`; +} + +type RangeSettingShape = { id: string; min: number; max: number; step: number }; + +function readRangeShape(setting: Setting.Any): RangeSettingShape | undefined { + if (!setting || typeof setting !== 'object') return; + if (!('type' in setting) || setting.type !== 'range') return; + if (!('id' in setting) || typeof setting.id !== 'string') return; + const { min, max, step } = setting as { min?: unknown; max?: unknown; step?: unknown }; + if (!isFiniteNumber(min) || !isFiniteNumber(max) || !isFiniteNumber(step)) return; + if (step <= 0) return; + return { id: setting.id, min, max, step }; +} + +export const ValidRangeStepCount: LiquidCheckDefinition = { + meta: { ...meta, type: SourceCodeType.LiquidHtml }, + + create(context) { + return { + async LiquidRawTag(node) { + if (node.name !== 'schema' || node.body.kind !== 'json') return; + + const schema = await getSchema(context); + const { validSchema, ast } = schema ?? {}; + if (!validSchema || validSchema instanceof Error) return; + if (!ast || ast instanceof Error) return; + if (!validSchema.settings) return; + + const offset = node.blockStartPosition.end; + + for (let i = 0; i < validSchema.settings.length; i++) { + const setting = validSchema.settings[i]; + const range = readRangeShape(setting); + if (!range) continue; + if (!exceedsMaxSteps(range.min, range.max, range.step)) continue; + + const stepNode = nodeAtPath(ast, ['settings', i, 'step']); + reportAtNode( + context, + offset, + stepNode, + tooManyStepsMessage(range.id, range.min, range.max, range.step), + ); + } + }, + }; + }, +}; + +function reportAtNode( + context: Parameters[0], + offset: number, + astNode: JSONNode | undefined, + message: string, +) { + if (!astNode) return; + context.report({ + message, + startIndex: offset + getLocStart(astNode), + endIndex: offset + getLocEnd(astNode), + }); +} + +export const ValidRangeStepCountSettingsSchema: JSONCheckDefinition = { + meta: { ...meta, type: SourceCodeType.JSON }, + + create(context) { + const relativePath = context.toRelativePath(context.file.uri); + if (relativePath !== 'config/settings_schema.json') return {}; + + return { + async Property(node) { + if (node.key.value !== 'settings' || !isArrayNode(node.value)) return; + + for (const settingNode of node.value.children) { + if (!isObjectNode(settingNode)) continue; + + const typeProp = settingNode.children.find((p) => p.key.value === 'type'); + if (!typeProp || !isLiteralNode(typeProp.value) || typeProp.value.value !== 'range') { + continue; + } + + const idProp = settingNode.children.find((p) => p.key.value === 'id'); + const idValue = + idProp && isLiteralNode(idProp.value) && typeof idProp.value.value === 'string' + ? idProp.value.value + : 'unknown'; + + const minProp = settingNode.children.find((p) => p.key.value === 'min'); + const maxProp = settingNode.children.find((p) => p.key.value === 'max'); + const stepProp = settingNode.children.find((p) => p.key.value === 'step'); + if (!minProp || !maxProp || !stepProp) continue; + if ( + !isLiteralNode(minProp.value) || + !isLiteralNode(maxProp.value) || + !isLiteralNode(stepProp.value) + ) { + continue; + } + + const min = minProp.value.value; + const max = maxProp.value.value; + const step = stepProp.value.value; + if (!isFiniteNumber(min) || !isFiniteNumber(max) || !isFiniteNumber(step)) continue; + if (step <= 0) continue; + if (!exceedsMaxSteps(min, max, step)) continue; + + context.report({ + message: tooManyStepsMessage(idValue, min, max, step), + startIndex: getLocStart(stepProp.value), + endIndex: getLocEnd(stepProp.value), + }); + } + }, + }; + }, +};