From f78191c9b5a0edba25a84b07f75cf76b3507ca32 Mon Sep 17 00:00:00 2001 From: SinhSinh An Date: Thu, 23 Apr 2026 15:31:01 -0500 Subject: [PATCH] feat(theme-check-common): add LiquidSchema check visitor method Introduces a `LiquidSchema(node)` method on `Check` that replaces the repeated preamble found in every schema-validating check: async LiquidRawTag(node) { if (node.name !== 'schema' || node.body.kind !== 'json') return; const offset = node.blockStartPosition.end; const schema = await getSchema(context); const { validSchema, ast } = schema ?? {}; if (!validSchema || validSchema instanceof Error) return; if (!ast || ast instanceof Error) return; // ... check logic ... } becomes: async LiquidSchema({ validSchema, ast, offset, schema }) { // ... check logic ... } Implementation: - New `LiquidSchemaNode` type exposing `{ node, schema, validSchema, ast, offset }`, with `validSchema` and `ast` narrowed to their non-Error variants. - New `wrapLiquidSchema` helper that transforms a check's `LiquidSchema` method into a composed `LiquidRawTag` visitor. This keeps `visitLiquid` oblivious to schemas and avoids threading `context` through the visitor. - `createCheck` runs `wrapLiquidSchema` once per check instance, so every LiquidHtml check gets the new method for free. - When a check declares both `LiquidRawTag` and `LiquidSchema`, the existing `LiquidRawTag` runs first for every raw tag, then the schema preamble runs and `LiquidSchema` fires only for valid schemas. - Checks not in section or theme-block files never fire `LiquidSchema` (matches the existing `getSchema` behaviour). Migrated `ValidSettingsKey` as a demonstration. Its 17 tests pass unchanged, proving the API is a drop-in replacement. Closes #817 --- .changeset/feat-liquid-schema-visitor.md | 9 + .../src/checks/valid-settings-key/index.ts | 13 +- packages/theme-check-common/src/index.ts | 5 +- packages/theme-check-common/src/types.ts | 78 ++++++++- .../src/wrap-liquid-schema.spec.ts | 162 ++++++++++++++++++ .../src/wrap-liquid-schema.ts | 83 +++++++++ 6 files changed, 336 insertions(+), 14 deletions(-) create mode 100644 .changeset/feat-liquid-schema-visitor.md create mode 100644 packages/theme-check-common/src/wrap-liquid-schema.spec.ts create mode 100644 packages/theme-check-common/src/wrap-liquid-schema.ts diff --git a/.changeset/feat-liquid-schema-visitor.md b/.changeset/feat-liquid-schema-visitor.md new file mode 100644 index 000000000..a5811c7a8 --- /dev/null +++ b/.changeset/feat-liquid-schema-visitor.md @@ -0,0 +1,9 @@ +--- +'@shopify/theme-check-common': minor +--- + +Add `LiquidSchema` check visitor method + +Liquid checks can now declare a `LiquidSchema(node)` method in place of the repeated `LiquidRawTag` + `getSchema` + `validSchema` / `ast` preamble. The method fires once per `{% schema %}` tag in a section or theme-block file, after the schema has been JSON-parsed and validated. The payload exposes `node`, `schema`, `validSchema`, `ast`, and `offset`, with `validSchema` and `ast` guaranteed to be non-Error. + +Existing `LiquidRawTag`-based checks continue to work. The two methods can be used side-by-side on the same check. 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..0740daa39 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 @@ -9,7 +9,7 @@ import { Setting, } from '../../types'; import { nodeAtPath } from '../../json'; -import { getSchema, isSectionSchema } from '../../to-schema'; +import { isSectionSchema } from '../../to-schema'; import { BlockDefNodeWithPath, getBlocks, reportWarning } from '../../utils'; export const ValidSettingsKey: LiquidCheckDefinition = { @@ -30,16 +30,7 @@ export const ValidSettingsKey: LiquidCheckDefinition = { create(context) { return { - async LiquidRawTag(node) { - if (node.name !== 'schema' || node.body.kind !== 'json') return; - - const offset = node.blockStartPosition.end; - const schema = await getSchema(context); - - const { validSchema, ast } = schema ?? {}; - if (!validSchema || validSchema instanceof Error) return; - if (!ast || ast instanceof Error) return; - + async LiquidSchema({ schema, validSchema, ast, offset }) { const { rootLevelLocalBlocks, presetLevelBlocks } = getBlocks(validSchema); // Check if presets settings match schema-level settings diff --git a/packages/theme-check-common/src/index.ts b/packages/theme-check-common/src/index.ts index df10c2c1a..97a85d0c8 100644 --- a/packages/theme-check-common/src/index.ts +++ b/packages/theme-check-common/src/index.ts @@ -37,6 +37,7 @@ import { } from './types'; import { getPosition } from './utils'; import { visitJSON, visitLiquid } from './visitors'; +import { wrapLiquidSchema } from './wrap-liquid-schema'; export * from './AbstractFileSystem'; export * from './AugmentedThemeDocset'; @@ -57,6 +58,7 @@ export * from './utils/memo'; export * from './utils/types'; export * from './utils/object'; export * from './visitor'; +export * from './wrap-liquid-schema'; export * from './liquid-doc/liquidDoc'; export { getBlockName } from './liquid-doc/arguments'; export * from './liquid-doc/utils'; @@ -192,7 +194,8 @@ function createCheck( validateJSON?: ValidateJSON, ): Check { const context = createContext(check, file, offenses, config, dependencies, validateJSON); - return check.create(context as any) as Check; + const instance = check.create(context as any) as Check; + return wrapLiquidSchema(instance, context); } function filesOfType(type: S, sourceCodes: Theme): SourceCode[] { diff --git a/packages/theme-check-common/src/types.ts b/packages/theme-check-common/src/types.ts index c5053c5a4..1d5394ddd 100644 --- a/packages/theme-check-common/src/types.ts +++ b/packages/theme-check-common/src/types.ts @@ -1,4 +1,9 @@ -import { LiquidHtmlNode, NodeTypes as LiquidHtmlNodeTypes } from '@shopify/liquid-html-parser'; +import { + LiquidHtmlNode, + LiquidRawTag, + NodeTypes as LiquidHtmlNodeTypes, +} from '@shopify/liquid-html-parser'; +import { Section, ThemeBlock } from './types/schemas'; import { Schema, Settings } from './types/schema-prop-factory'; @@ -245,9 +250,78 @@ export type CheckDefinition< * } */ export type Check = T extends SourceCodeType - ? Partial & CheckExitMethods & CheckLifecycleMethods> + ? Partial< + CheckNodeMethods & + CheckExitMethods & + CheckLifecycleMethods & + CheckExtraMethods + > : never; +/** + * Payload for the synthetic `LiquidSchema` check method. + * + * Fires once per `{% schema %}` tag in a section or theme-block file, after + * the tag has been schema-validated. Abstracts the repeated preamble of + * filtering for `name === 'schema'`, calling `getSchema(context)`, and + * guarding against `undefined` / `Error` results. + */ +export interface LiquidSchemaNode { + /** The original `{% schema %}` LiquidRawTag node. */ + node: LiquidRawTag; + /** + * The full schema object (`SectionSchema` or `ThemeBlockSchema`). + * Useful with `isSectionSchema(schema)` / `isBlockSchema(schema)` for + * type narrowing between section and block schemas. + */ + schema: SectionSchema | ThemeBlockSchema; + /** The validated, strongly-typed schema contents. Never an Error. */ + validSchema: Section.Schema | ThemeBlock.Schema; + /** The JSON AST for the schema body. Never an Error. */ + ast: JSONNode; + /** + * Character offset within the Liquid source where the JSON begins. + * Add this to JSON node positions to get document-level offsets when + * calling `context.report({ startIndex, endIndex })`. + */ + offset: number; +} + +/** Extra non-node-type-based visitor methods a check can declare. */ +type CheckExtraMethods = T extends SourceCodeType.LiquidHtml + ? { + /** + * Called once per `{% schema %}` tag in a section or theme-block file, + * after the tag body has been parsed and schema-validated. + * + * Replaces the common preamble: + * + * ```ts + * 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; + * // ... + * } + * ``` + * + * with: + * + * ```ts + * async LiquidSchema({ validSchema, ast, offset }) { + * // ... + * } + * ``` + */ + LiquidSchema: ( + node: LiquidSchemaNode, + ancestors: LiquidHtmlNode[], + ) => Promise; + } + : {}; + export type CheckNodeMethod = ( node: NodeOfType, ancestors: AST[T][], diff --git a/packages/theme-check-common/src/wrap-liquid-schema.spec.ts b/packages/theme-check-common/src/wrap-liquid-schema.spec.ts new file mode 100644 index 000000000..7d6fdfb49 --- /dev/null +++ b/packages/theme-check-common/src/wrap-liquid-schema.spec.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from 'vitest'; +import { check } from './test'; +import { + LiquidCheckDefinition, + LiquidSchemaNode, + Severity, + SourceCodeType, +} from './types'; + +function makeSchemaCapturingCheck(captured: LiquidSchemaNode[]): LiquidCheckDefinition { + return { + meta: { + code: 'SchemaSpyCheck', + name: 'Schema Spy', + docs: { + description: 'Test check that captures every LiquidSchema invocation.', + recommended: true, + url: 'https://example.com', + }, + type: SourceCodeType.LiquidHtml, + severity: Severity.ERROR, + schema: {}, + targets: [], + }, + create() { + return { + async LiquidSchema(node) { + captured.push(node); + }, + }; + }, + }; +} + +describe('Module: wrapLiquidSchema', () => { + it('fires LiquidSchema once per valid {% schema %} tag in a section', async () => { + const captured: LiquidSchemaNode[] = []; + await check( + { + 'sections/hero.liquid': ` +
+ {% schema %} + { + "name": "Hero", + "settings": [] + } + {% endschema %} + `, + }, + [makeSchemaCapturingCheck(captured)], + ); + + expect(captured).toHaveLength(1); + expect(captured[0].validSchema).not.toBeInstanceOf(Error); + expect(captured[0].ast).not.toBeInstanceOf(Error); + expect(captured[0].schema.type).toBe('section'); + expect(captured[0].schema.name).toBe('hero'); + expect(captured[0].validSchema.name).toBe('Hero'); + expect(captured[0].offset).toBe(captured[0].node.blockStartPosition.end); + }); + + it('fires LiquidSchema for theme blocks', async () => { + const captured: LiquidSchemaNode[] = []; + await check( + { + 'blocks/card.liquid': ` +
+ {% schema %} + { + "name": "Card" + } + {% endschema %} + `, + }, + [makeSchemaCapturingCheck(captured)], + ); + + expect(captured).toHaveLength(1); + expect(captured[0].schema.type).toBe('block'); + expect(captured[0].schema.name).toBe('card'); + }); + + it('does not fire LiquidSchema in snippet files', async () => { + const captured: LiquidSchemaNode[] = []; + await check( + { + 'snippets/utils.liquid': ` + {% schema %} + { "name": "Oops" } + {% endschema %} + `, + }, + [makeSchemaCapturingCheck(captured)], + ); + + expect(captured).toHaveLength(0); + }); + + it('does not fire LiquidSchema for non-schema raw tags', async () => { + const captured: LiquidSchemaNode[] = []; + await check( + { + 'sections/hero.liquid': ` + {% stylesheet %} + .hero { color: red; } + {% endstylesheet %} + `, + }, + [makeSchemaCapturingCheck(captured)], + ); + + expect(captured).toHaveLength(0); + }); + + it('composes with an existing LiquidRawTag method', async () => { + const rawTagNames: string[] = []; + const schemaHits: LiquidSchemaNode[] = []; + + const composedCheck: LiquidCheckDefinition = { + meta: { + code: 'ComposedSpyCheck', + name: 'Composed Spy', + docs: { + description: 'Test check that uses both LiquidRawTag and LiquidSchema.', + recommended: true, + url: 'https://example.com', + }, + type: SourceCodeType.LiquidHtml, + severity: Severity.ERROR, + schema: {}, + targets: [], + }, + create() { + return { + async LiquidRawTag(node) { + rawTagNames.push(node.name); + }, + async LiquidSchema(node) { + schemaHits.push(node); + }, + }; + }, + }; + + await check( + { + 'sections/hero.liquid': ` + {% stylesheet %}.x{}{% endstylesheet %} + {% schema %} + { "name": "Hero" } + {% endschema %} + `, + }, + [composedCheck], + ); + + // Both raw tags visited; only the schema tag fired LiquidSchema + expect(rawTagNames).toEqual(expect.arrayContaining(['stylesheet', 'schema'])); + expect(schemaHits).toHaveLength(1); + expect(schemaHits[0].schema.name).toBe('hero'); + }); +}); diff --git a/packages/theme-check-common/src/wrap-liquid-schema.ts b/packages/theme-check-common/src/wrap-liquid-schema.ts new file mode 100644 index 000000000..cb153fc44 --- /dev/null +++ b/packages/theme-check-common/src/wrap-liquid-schema.ts @@ -0,0 +1,83 @@ +import { LiquidHtmlNode, LiquidRawTag } from '@shopify/liquid-html-parser'; +import { getSchema } from './to-schema'; +import { + Check, + Context, + LiquidSchemaNode, + Schema, + SectionSchema, + SourceCodeType, + ThemeBlockSchema, +} from './types'; + +/** + * Transforms a LiquidHtml check's optional `LiquidSchema` method into a + * standard `LiquidRawTag` visitor that bakes in the schema-loading preamble. + * + * Users can declare a `LiquidSchema` method on their check: + * + * ```ts + * create(context) { + * return { + * async LiquidSchema({ validSchema, ast, offset }) { + * // runs only after getSchema() resolved and validSchema/ast + * // are both non-Error + * }, + * }; + * } + * ``` + * + * and this helper composes it with any existing `LiquidRawTag` method so + * the core visitor (`visitLiquid`) does not need to know about schemas. + * + * If the check does not declare `LiquidSchema`, returns the check unchanged. + * If the check declares both `LiquidSchema` and `LiquidRawTag`, the + * existing `LiquidRawTag` method runs first for every raw tag, then the + * schema preamble runs and `LiquidSchema` fires only for valid schemas. + */ +export function wrapLiquidSchema( + check: Check, + context: Context, +): Check { + if (context.file.type !== SourceCodeType.LiquidHtml) return check; + + const liquidCheck = check as Check; + const schemaMethod = liquidCheck.LiquidSchema; + if (!schemaMethod) return check; + + const existingLiquidRawTag = liquidCheck.LiquidRawTag; + const liquidContext = context as unknown as Context; + + const wrappedLiquidRawTag = async ( + node: LiquidRawTag, + ancestors: LiquidHtmlNode[], + ): Promise => { + if (existingLiquidRawTag) { + await existingLiquidRawTag(node, ancestors); + } + + if (node.name !== 'schema' || node.body.kind !== 'json') return; + + const schema = await getSchema(liquidContext); + if (!schema) return; + + const { validSchema, ast } = schema; + if (!validSchema || validSchema instanceof Error) return; + if (!ast || ast instanceof Error) return; + + const payload: LiquidSchemaNode = { + node, + schema: schema as SectionSchema | ThemeBlockSchema, + validSchema, + ast, + offset: node.blockStartPosition.end, + }; + + await schemaMethod(payload, ancestors); + }; + + return { + ...liquidCheck, + LiquidRawTag: wrappedLiquidRawTag, + } as Check; +}