From 54e07d372792a7da9b63daac7104f97807774de4 Mon Sep 17 00:00:00 2001 From: Cristofer Hippleheuser Date: Sun, 21 Jun 2026 23:45:30 -0500 Subject: [PATCH] Validate section setting keys in JSON templates --- .changeset/bright-keys-check.md | 5 + .../theme-check-common/src/checks/index.ts | 3 +- .../checks/valid-settings-key/index.spec.ts | 252 +++++++++++++++++- .../src/checks/valid-settings-key/index.ts | 158 +++++++++-- 4 files changed, 400 insertions(+), 18 deletions(-) create mode 100644 .changeset/bright-keys-check.md diff --git a/.changeset/bright-keys-check.md b/.changeset/bright-keys-check.md new file mode 100644 index 000000000..996b7ea0f --- /dev/null +++ b/.changeset/bright-keys-check.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': minor +--- + +Validate section setting keys in JSON templates. diff --git a/packages/theme-check-common/src/checks/index.ts b/packages/theme-check-common/src/checks/index.ts index be0370c41..f750d6428 100644 --- a/packages/theme-check-common/src/checks/index.ts +++ b/packages/theme-check-common/src/checks/index.ts @@ -62,7 +62,7 @@ import { ValidRenderSnippetArgumentTypes } from './valid-render-snippet-argument import { ValidSchema } from './valid-schema'; import { ValidSchemaName } from './valid-schema-name'; import { ValidSchemaTranslations } from './valid-schema-translations'; -import { ValidSettingsKey } from './valid-settings-key'; +import { ValidSettingsKey, ValidSettingsKeyJSON } from './valid-settings-key'; import { ValidStaticBlockType } from './valid-static-block-type'; import { ValidVisibleIf, ValidVisibleIfSettingsSchema } from './valid-visible-if'; import { VariableName } from './variable-name'; @@ -133,6 +133,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [ ValidRenderSnippetArgumentTypes, ValidSchema, ValidSettingsKey, + ValidSettingsKeyJSON, ValidStaticBlockType, ValidVisibleIf, ValidVisibleIfSettingsSchema, diff --git a/packages/theme-check-common/src/checks/valid-settings-key/index.spec.ts b/packages/theme-check-common/src/checks/valid-settings-key/index.spec.ts index 69bccea7c..f0fccf9cd 100644 --- a/packages/theme-check-common/src/checks/valid-settings-key/index.spec.ts +++ b/packages/theme-check-common/src/checks/valid-settings-key/index.spec.ts @@ -1,6 +1,6 @@ import { expect, describe, it } from 'vitest'; import { check } from '../../test'; -import { ValidSettingsKey } from './index'; +import { ValidSettingsKey, ValidSettingsKeyJSON } from './index'; describe('Module: ValidSettingsKey', () => { const schemaTemplate = { @@ -116,6 +116,256 @@ describe('Module: ValidSettingsKey', () => { }); }); + describe('template JSON section settings', () => { + it('does not report an error when a template JSON section setting exists in the section schema', async () => { + const theme = { + 'templates/page.json': JSON.stringify({ + sections: { + 'main-page': { + type: 'main-page', + settings: { + heading_tag: 'h1', + }, + }, + }, + order: ['main-page'], + }), + 'sections/main-page.liquid': toLiquidFile({ + name: 'Main page', + settings: [ + { + id: 'heading_tag', + type: 'select', + label: 'Heading tag', + options: [ + { value: 'h1', label: 'H1' }, + { value: 'h2', label: 'H2' }, + ], + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey, ValidSettingsKeyJSON]); + expect(offenses).to.have.length(0); + }); + + it('reports an error when a template JSON section setting does not exist in the section schema', async () => { + const theme = { + 'templates/page.json': JSON.stringify({ + sections: { + 'main-page': { + type: 'main-page', + settings: { + heading_tagg: 'h1', + }, + }, + }, + order: ['main-page'], + }), + 'sections/main-page.liquid': toLiquidFile({ + name: 'Main page', + settings: [ + { + id: 'heading_tag', + type: 'select', + label: 'Heading tag', + options: [ + { value: 'h1', label: 'H1' }, + { value: 'h2', label: 'H2' }, + ], + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey, ValidSettingsKeyJSON]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + `Setting 'heading_tagg' does not exist in 'sections/main-page.liquid'.`, + ); + }); + }); + + describe('template JSON block settings', () => { + it('does not report an error when a template JSON block setting exists in the block schema', async () => { + const theme = { + 'templates/page.json': JSON.stringify({ + sections: { + 'main-page': { + type: 'main-page', + blocks: { + text: { + type: 'text', + settings: { + heading: 'Hello', + }, + }, + }, + block_order: ['text'], + }, + }, + order: ['main-page'], + }), + 'sections/main-page.liquid': toLiquidFile({ + name: 'Main page', + blocks: [{ type: 'text' }], + }), + 'blocks/text.liquid': toLiquidFile({ + name: 'Text', + settings: [ + { + id: 'heading', + type: 'text', + label: 'Heading', + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey, ValidSettingsKeyJSON]); + expect(offenses).to.have.length(0); + }); + + it('reports an error when a template JSON block setting does not exist in the block schema', async () => { + const theme = { + 'templates/page.json': JSON.stringify({ + sections: { + 'main-page': { + type: 'main-page', + blocks: { + text: { + type: 'text', + settings: { + headingg: 'Hello', + }, + }, + }, + block_order: ['text'], + }, + }, + order: ['main-page'], + }), + 'sections/main-page.liquid': toLiquidFile({ + name: 'Main page', + blocks: [{ type: 'text' }], + }), + 'blocks/text.liquid': toLiquidFile({ + name: 'Text', + settings: [ + { + id: 'heading', + type: 'text', + label: 'Heading', + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey, ValidSettingsKeyJSON]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + `Setting 'headingg' does not exist in 'blocks/text.liquid'.`, + ); + }); + + it('reports an error when a template JSON local block setting does not exist in the section schema', async () => { + const theme = { + 'templates/page.json': JSON.stringify({ + sections: { + 'main-page': { + type: 'main-page', + blocks: { + foo: { + type: 'foo', + settings: { + headingg: 'Hello', + }, + }, + }, + block_order: ['foo'], + }, + }, + order: ['main-page'], + }), + 'sections/main-page.liquid': toLiquidFile({ + name: 'Main page', + blocks: [ + { + type: 'foo', + name: 'Foo', + settings: [ + { + id: 'heading', + type: 'text', + label: 'Heading', + }, + ], + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey, ValidSettingsKeyJSON]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + `Setting 'headingg' does not exist in 'sections/main-page.liquid'.`, + ); + }); + + it('reports an error when a nested template JSON block setting does not exist in the nested block schema', async () => { + const theme = { + 'templates/page.json': JSON.stringify({ + sections: { + 'main-page': { + type: 'main-page', + blocks: { + parent: { + type: 'parent', + blocks: { + text: { + type: 'text', + settings: { + headingg: 'Hello', + }, + }, + }, + block_order: ['text'], + }, + }, + block_order: ['parent'], + }, + }, + order: ['main-page'], + }), + 'sections/main-page.liquid': toLiquidFile({ + name: 'Main page', + blocks: [{ type: 'parent' }], + }), + 'blocks/parent.liquid': toLiquidFile({ + name: 'Parent', + blocks: [{ type: 'text' }], + }), + 'blocks/text.liquid': toLiquidFile({ + name: 'Text', + settings: [ + { + id: 'heading', + type: 'text', + label: 'Heading', + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey, ValidSettingsKeyJSON]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + `Setting 'headingg' does not exist in 'blocks/text.liquid'.`, + ); + }); + }); + const tests = [ { label: 'default', diff --git a/packages/theme-check-common/src/checks/valid-settings-key/index.ts b/packages/theme-check-common/src/checks/valid-settings-key/index.ts index 0fbaebb53..4b36f35be 100644 --- a/packages/theme-check-common/src/checks/valid-settings-key/index.ts +++ b/packages/theme-check-common/src/checks/valid-settings-key/index.ts @@ -1,32 +1,35 @@ import { + JSONCheckDefinition, LiquidCheckDefinition, Severity, SourceCodeType, Preset, Section, ThemeBlock, + Template, JSONNode, Setting, } from '../../types'; import { nodeAtPath } from '../../json'; -import { getSchema, isSectionSchema } from '../../to-schema'; +import { getSchema, getSchemaFromJSON, isSectionSchema } from '../../to-schema'; import { BlockDefNodeWithPath, getBlocks, reportWarning } from '../../utils'; -export const ValidSettingsKey: LiquidCheckDefinition = { - meta: { - code: 'ValidSettingsKey', - name: 'Validate settings key in presets', - docs: { - description: - 'Ensures settings key only references valid settings defined in its respective schema', - recommended: true, - url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/valid-settings-key', - }, - type: SourceCodeType.LiquidHtml, - severity: Severity.ERROR, - schema: {}, - targets: [], +const meta = { + code: 'ValidSettingsKey', + name: 'Validate settings key in presets', + docs: { + description: + 'Ensures settings key only references valid settings defined in its respective schema', + recommended: true, + url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/valid-settings-key', }, + severity: Severity.ERROR, + schema: {}, + targets: [], +}; + +export const ValidSettingsKey: LiquidCheckDefinition = { + meta: { ...meta, type: SourceCodeType.LiquidHtml }, create(context) { return { @@ -78,6 +81,61 @@ export const ValidSettingsKey: LiquidCheckDefinition = { }, }; +export const ValidSettingsKeyJSON: JSONCheckDefinition = { + meta: { ...meta, type: SourceCodeType.JSON }, + + create(context) { + const relativePath = context.toRelativePath(context.file.uri); + if (!relativePath.startsWith('templates/')) return {}; + + return { + async onCodePathEnd() { + const schema = await getSchemaFromJSON(context); + const { ast } = schema ?? {}; + if (!schema || !ast || ast instanceof Error) return; + + const sections = schema.parsed.sections; + if (!sections) return; + + await Promise.all( + Object.entries(sections).map(async ([sectionKey, section]) => { + if (!isPropertyNode(section) || !('type' in section)) { + return; + } + + const sectionSchema = await context.getSectionSchema?.(section.type); + const { validSchema } = sectionSchema ?? {}; + if (!validSchema || validSchema instanceof Error) return; + + if ('settings' in section) { + const settingsNode = nodeAtPath(ast, ['sections', sectionKey, 'settings']); + validateSettingsKey( + context, + 0, + settingsNode, + validSchema.settings, + undefined, + `'sections/${section.type}.liquid'`, + ); + } + + if ('blocks' in section && isPropertyNode(section.blocks)) { + await validateTemplateBlocksSettings( + context, + ast, + section.blocks, + validSchema, + ['sections', sectionKey, 'blocks'], + `'sections/${section.type}.liquid'`, + ); + } + }), + ); + }, + }; + }, +}; + async function validateReferencedBlock( context: any, offset: number, @@ -104,12 +162,76 @@ async function validateReferencedBlock( } } +async function validateTemplateBlocksSettings( + context: any, + ast: JSONNode, + blocks: Record, + parentSchema: Section.Schema | ThemeBlock.Schema, + currentPath: string[], + parentSchemaPath: string, +) { + await Promise.all( + Object.entries(blocks).map(async ([blockKey, block]) => { + if (!isPropertyNode(block) || !('type' in block)) return; + + const currentBlockPath = currentPath.concat(blockKey); + const localBlock = parentSchema.blocks?.find( + (schemaBlock) => schemaBlock.type === block.type && 'name' in schemaBlock, + ) as Section.LocalBlock | undefined; + + if (localBlock) { + const settingsNode = nodeAtPath(ast, currentBlockPath.concat('settings')); + validateSettingsKey( + context, + 0, + settingsNode, + localBlock.settings, + undefined, + parentSchemaPath, + ); + + if ('blocks' in block && isPropertyNode(block.blocks)) { + await validateTemplateBlocksSettings( + context, + ast, + block.blocks, + localBlock, + currentBlockPath.concat('blocks'), + parentSchemaPath, + ); + } + + return; + } + + const blockSchema = await context.getBlockSchema?.(block.type); + const { validSchema } = blockSchema ?? {}; + if (!validSchema || validSchema instanceof Error) return; + + const settingsNode = nodeAtPath(ast, currentBlockPath.concat('settings')); + validateSettingsKey(context, 0, settingsNode, validSchema.settings, block); + + if ('blocks' in block && isPropertyNode(block.blocks)) { + await validateTemplateBlocksSettings( + context, + ast, + block.blocks, + validSchema, + currentBlockPath.concat('blocks'), + `'blocks/${block.type}.liquid'`, + ); + } + }), + ); +} + function validateSettingsKey( context: any, offset: number, settingsNode: JSONNode | undefined, validSettings: Setting.Any[] | undefined, blockNode?: Section.Block | ThemeBlock.Block | Preset.Block, + schemaPath = 'schema', ) { if (!settingsNode || settingsNode.type !== 'Object') return; @@ -121,9 +243,13 @@ function validateSettingsKey( if (!settingExists) { const errorMessage = blockNode ? `Setting '${setting.key.value}' does not exist in 'blocks/${blockNode.type}.liquid'.` - : `Setting '${setting.key.value}' does not exist in schema.`; + : `Setting '${setting.key.value}' does not exist in ${schemaPath}.`; reportWarning(errorMessage, offset, setting.key, context); } } } + +function isPropertyNode(node: unknown): node is Record { + return typeof node === 'object' && node !== null; +}