Skip to content

feat(theme-check-common): two checks for range setting structural validity #1202

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.

Sign up for GitHub

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

Open
hyldmo wants to merge 1 commit into
base: main
Choose a base branch
from
+992 −0
Open

feat(theme-check-common): two checks for range setting structural validity #1202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions .changeset/valid-range-default.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions packages/theme-check-common/src/checks/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -130,6 +132,10 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
ValidJSON,
ValidDocParamTypes,
ValidLocalBlocks,
ValidRangeDefault,
ValidRangeDefaultSettingsSchema,
ValidRangeStepCount,
ValidRangeStepCountSettingsSchema,
ValidRenderSnippetArgumentTypes,
ValidSchema,
ValidSettingsKey,
Expand Down
355 changes: 355 additions & 0 deletions packages/theme-check-common/src/checks/valid-range-default/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading
Loading