From 3799990abdeafb804cc53064b7ae300a007b376a Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Sat, 12 Sep 2026 13:34:51 +0200 Subject: [PATCH 1/6] Allow high-priority image preloads --- .changeset/fix-responsive-lcp-preloads.md | 5 ++++ .../src/checks/asset-preload/index.spec.ts | 12 +++++++++ .../src/checks/asset-preload/index.ts | 25 ++++++++++++++++--- 3 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-responsive-lcp-preloads.md diff --git a/.changeset/fix-responsive-lcp-preloads.md b/.changeset/fix-responsive-lcp-preloads.md new file mode 100644 index 000000000..5b690ab95 --- /dev/null +++ b/.changeset/fix-responsive-lcp-preloads.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +Fix responsive LCP preload validation by allowing explicit image links with `fetchpriority="high"`. diff --git a/packages/theme-check-common/src/checks/asset-preload/index.spec.ts b/packages/theme-check-common/src/checks/asset-preload/index.spec.ts index a9f8b4494..e746309be 100644 --- a/packages/theme-check-common/src/checks/asset-preload/index.spec.ts +++ b/packages/theme-check-common/src/checks/asset-preload/index.spec.ts @@ -43,6 +43,18 @@ describe('Module: AssetPreload', () => { expect(highlights).to.eql([``]); }); + it('allows high-priority image preloading', async () => { + // The preload_tag filter cannot generate responsive imagesrcset or + // imagesizes attributes, so LCP images may require an explicit + // high-priority preload link. + const sourceCode = ` + + `; + + const offenses = await runLiquidCheck(AssetPreload, sourceCode); + expect(offenses).to.have.lengthOf(0); + }); + it('reports general preloading', async () => { const sourceCode = ` diff --git a/packages/theme-check-common/src/checks/asset-preload/index.ts b/packages/theme-check-common/src/checks/asset-preload/index.ts index 6e13d8faa..a3220b633 100644 --- a/packages/theme-check-common/src/checks/asset-preload/index.ts +++ b/packages/theme-check-common/src/checks/asset-preload/index.ts @@ -9,6 +9,21 @@ function isPreload(attr: ValuedHtmlAttribute): boolean { ); } +function isHighPriorityImagePreload(attributes: ValuedHtmlAttribute[]): boolean { + return ( + attributes.some( + (attr) => + isAttr(attr, 'as') && + attr.value.some((node) => node.type === NodeTypes.TextNode && node.value === 'image'), + ) && + attributes.some( + (attr) => + isAttr(attr, 'fetchpriority') && + attr.value.some((node) => node.type === NodeTypes.TextNode && node.value === 'high'), + ) + ); +} + export const AssetPreload: LiquidCheckDefinition = { meta: { code: 'AssetPreload', @@ -33,9 +48,13 @@ export const AssetPreload: LiquidCheckDefinition = { ) as ValuedHtmlAttribute | undefined; if (node.name === 'link' && preloadLinkAttr) { - const asAttr: ValuedHtmlAttribute | undefined = node.attributes - .filter(isValuedHtmlAttribute) - .find((attr) => isAttr(attr, 'as')); + const valuedAttributes = node.attributes.filter(isValuedHtmlAttribute); + + if (isHighPriorityImagePreload(valuedAttributes)) return; + + const asAttr: ValuedHtmlAttribute | undefined = valuedAttributes.find((attr) => + isAttr(attr, 'as'), + ); const assetType = asAttr?.value.find((node): node is TextNode => isNodeOfType(NodeTypes.TextNode, node), From 3f82bb5b84957f13d644fbb7b6e957128485b8f2 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Sat, 12 Sep 2026 15:05:41 +0200 Subject: [PATCH 2/6] Reserve Liquid literal parameter names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are meaningful because Liquid always parses them as literals. For example, an @param true value could be passed but never referenced as a variable—{{ true }} is always Boolean true. --- .../fix-liquid-doc-literal-parameters.md | 5 + .../reserved-doc-param-names/index.spec.ts | 93 ++++++++----------- .../checks/reserved-doc-param-names/index.ts | 40 ++------ 3 files changed, 56 insertions(+), 82 deletions(-) create mode 100644 .changeset/fix-liquid-doc-literal-parameters.md diff --git a/.changeset/fix-liquid-doc-literal-parameters.md b/.changeset/fix-liquid-doc-literal-parameters.md new file mode 100644 index 000000000..c835665ec --- /dev/null +++ b/.changeset/fix-liquid-doc-literal-parameters.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +Fix LiquidDoc parameter validation by reserving Liquid literal keywords. diff --git a/packages/theme-check-common/src/checks/reserved-doc-param-names/index.spec.ts b/packages/theme-check-common/src/checks/reserved-doc-param-names/index.spec.ts index 5f8b7d470..df1122605 100644 --- a/packages/theme-check-common/src/checks/reserved-doc-param-names/index.spec.ts +++ b/packages/theme-check-common/src/checks/reserved-doc-param-names/index.spec.ts @@ -3,60 +3,49 @@ import { ReservedDocParamNames } from './index'; import { runLiquidCheck } from '../../test'; describe('Module: ReservedDocParamNames', () => { - describe('block file', () => { - it(`should not report an error when no doc params share names with reserved content_for tag params`, async () => { + it.each(['blocks/file.liquid', 'snippets/file.liquid'])( + 'reports Liquid literal parameter names in %s', + async (fileName) => { const sourceCode = ` - {% doc %} - @param param1 - Example param - {% enddoc %} - `; - - const offenses = await runLiquidCheck( - ReservedDocParamNames, - sourceCode, - 'blocks/file.liquid', - ); - - expect(offenses).to.be.empty; - }); - - it('should report an error when a doc param shares names with reserved content_for tag params', async () => { + {% doc %} + @param nil - Example param + @param null - Example param + @param true - Example param + @param false - Example param + @param blank - Example param + @param empty - Example param + {% enddoc %} + `; + + const offenses = await runLiquidCheck(ReservedDocParamNames, sourceCode, fileName); + + expect(offenses).to.have.length(6); + expect(offenses.map(({ message }) => message)).toEqual([ + "The parameter name 'nil' is reserved because Liquid parses it as a literal.", + "The parameter name 'null' is reserved because Liquid parses it as a literal.", + "The parameter name 'true' is reserved because Liquid parses it as a literal.", + "The parameter name 'false' is reserved because Liquid parses it as a literal.", + "The parameter name 'blank' is reserved because Liquid parses it as a literal.", + "The parameter name 'empty' is reserved because Liquid parses it as a literal.", + ]); + }, + ); + + it.each(['blocks/file.liquid', 'snippets/file.liquid'])( + 'allows non-literal parameter names in %s', + async (fileName) => { const sourceCode = ` - {% doc %} - @param param1 - Example param - @param id - Example param - {% enddoc %} - `; - - const offenses = await runLiquidCheck( - ReservedDocParamNames, - sourceCode, - 'blocks/file.liquid', - ); + {% doc %} + @param param1 - Example param + @param class - Example param + @param attributes - Example param + @param id - Example param + {% enddoc %} + `; - expect(offenses).to.have.length(1); - expect(offenses[0].message).to.contain( - `The parameter name is not supported because it's a reserved argument for 'content_for' tags.`, - ); - }); - }); + const offenses = await runLiquidCheck(ReservedDocParamNames, sourceCode, fileName); - describe('snippet file', () => { - it('should not report an error when a doc param shares names with reserved content_for tag params', async () => { - const sourceCode = ` - {% doc %} - @param param1 - Example param - @param id - Example param - {% enddoc %} - `; - - const offenses = await runLiquidCheck( - ReservedDocParamNames, - sourceCode, - 'snippets/file.liquid', - ); - - expect(offenses).to.have.length(0); - }); - }); + expect(offenses).to.be.empty; + }, + ); }); diff --git a/packages/theme-check-common/src/checks/reserved-doc-param-names/index.ts b/packages/theme-check-common/src/checks/reserved-doc-param-names/index.ts index 764371e50..89412e334 100644 --- a/packages/theme-check-common/src/checks/reserved-doc-param-names/index.ts +++ b/packages/theme-check-common/src/checks/reserved-doc-param-names/index.ts @@ -1,10 +1,7 @@ -import { TextNode } from '@shopify/liquid-html-parser'; +import { LiquidLiteralValues } from '@shopify/liquid-html-parser'; import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; -import { isBlock } from '../../to-schema'; -import { - REQUIRED_CONTENT_FOR_ARGUMENTS, - RESERVED_CONTENT_FOR_ARGUMENTS, -} from '../../tags/content-for'; + +const RESERVED_DOC_PARAM_NAMES = new Set(Object.keys(LiquidLiteralValues)); export const ReservedDocParamNames: LiquidCheckDefinition = { meta: { @@ -23,35 +20,18 @@ export const ReservedDocParamNames: LiquidCheckDefinition = { }, create(context) { - if (!isBlock(context.file.uri)) { - return {}; - } - - const defaultParameterNames = [ - ...REQUIRED_CONTENT_FOR_ARGUMENTS, - ...RESERVED_CONTENT_FOR_ARGUMENTS, - ]; - return { async LiquidDocParamNode(node) { const paramName = node.paramName.value; - if (defaultParameterNames.includes(paramName)) { - reportWarning( - context, - `The parameter name is not supported because it's a reserved argument for 'content_for' tags.`, - node.paramName, - ); - } + if (!RESERVED_DOC_PARAM_NAMES.has(paramName)) return; + + context.report({ + message: `The parameter name '${paramName}' is reserved because Liquid parses it as a literal.`, + startIndex: node.paramName.position.start, + endIndex: node.paramName.position.end, + }); }, }; }, }; - -function reportWarning(context: any, message: string, node: TextNode) { - context.report({ - message, - startIndex: node.position.start, - endIndex: node.position.end, - }); -} From fe288d8c2fd7789215e5c125c961c60f46a56c9c Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Tue, 15 Sep 2026 09:29:16 +0200 Subject: [PATCH 3/6] Support assigned variables in schema expressions Allow ValidVisibleIf to resolve variables assigned by the Liquid file, and count references from valid visible_if expressions when checking for unused assignments. --- .changeset/fix-assigned-schema-variables.md | 5 +++ .../src/checks/unused-assign/index.spec.ts | 42 +++++++++++++++++++ .../src/checks/unused-assign/index.ts | 21 ++++++++++ .../src/checks/valid-visible-if/index.spec.ts | 20 +++++++++ .../src/checks/valid-visible-if/index.ts | 23 +++++++++- 5 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-assigned-schema-variables.md diff --git a/.changeset/fix-assigned-schema-variables.md b/.changeset/fix-assigned-schema-variables.md new file mode 100644 index 000000000..e53ce1cc6 --- /dev/null +++ b/.changeset/fix-assigned-schema-variables.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +Fix schema expressions to resolve assigned Liquid variables and recognize their usage. diff --git a/packages/theme-check-common/src/checks/unused-assign/index.spec.ts b/packages/theme-check-common/src/checks/unused-assign/index.spec.ts index f5575077a..109470280 100644 --- a/packages/theme-check-common/src/checks/unused-assign/index.spec.ts +++ b/packages/theme-check-common/src/checks/unused-assign/index.spec.ts @@ -114,6 +114,48 @@ describe('Module: UnusedAssign', () => { } }); + it('should not report variables used by a schema visible_if expression', async () => { + const sourceCode = ` + {% assign has_logo_image = true %} + {% schema %} + { + "name": "Logo", + "settings": [ + { + "type": "range", + "id": "height", + "min": 16, + "max": 80, + "step": 4, + "default": 32, + "visible_if": "{{ has_logo_image == true }}" + } + ] + } + {% endschema %} + `; + + const offenses = await runLiquidCheck(UnusedAssign, sourceCode, 'blocks/logo.liquid'); + + expect(offenses).to.be.empty; + }); + + it('should still report variables mentioned only in other schema strings', async () => { + const sourceCode = ` + {% assign unused_var = true %} + {% schema %} + { + "name": "unused_var", + "settings": [] + } + {% endschema %} + `; + + const offenses = await runLiquidCheck(UnusedAssign, sourceCode, 'blocks/example.liquid'); + + expect(offenses).to.have.lengthOf(1); + }); + it('should not report unused assigns for things used in a HTML raw-like tag', async () => { const tags = ['style', 'script']; for (const tag of tags) { diff --git a/packages/theme-check-common/src/checks/unused-assign/index.ts b/packages/theme-check-common/src/checks/unused-assign/index.ts index 10065060e..cf7d82ef2 100644 --- a/packages/theme-check-common/src/checks/unused-assign/index.ts +++ b/packages/theme-check-common/src/checks/unused-assign/index.ts @@ -6,7 +6,9 @@ import { NodeTypes, } from '@shopify/liquid-html-parser'; import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; +import { getSchema } from '../../to-schema'; import { isWithinRawTagThatDoesNotParseItsContents } from '../utils'; +import { getVariableLookupsInExpression } from '../valid-visible-if/visible-if-utils'; export const UnusedAssign: LiquidCheckDefinition = { meta: { @@ -53,6 +55,25 @@ export const UnusedAssign: LiquidCheckDefinition = { checkVariableUsage(node); }, + async LiquidRawTag(node) { + if (node.name !== 'schema' || node.body.kind !== 'json') return; + + const schema = await getSchema(context); + const validSchema = schema?.validSchema; + if (!validSchema || validSchema instanceof Error) return; + + for (const setting of validSchema.settings ?? []) { + if (!('visible_if' in setting) || typeof setting.visible_if !== 'string') continue; + + const lookups = getVariableLookupsInExpression(setting.visible_if); + if (!lookups || 'warning' in lookups) continue; + + for (const lookup of lookups) { + if (lookup.name) usedVariables.add(lookup.name); + } + } + }, + async onCodePathEnd() { for (const [variable, node] of assignedVariables.entries()) { if (!usedVariables.has(variable) && !variable.startsWith('_')) { diff --git a/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts b/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts index 65a23cee1..4555b6f91 100644 --- a/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts +++ b/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts @@ -97,6 +97,26 @@ describe('Module: ValidVisibleIf', () => { expect(offenses).toEqual([]); }); + it('reports no error for a variable assigned in the Liquid file', async () => { + const themeData = structuredClone(baseThemeData); + + themeData['blocks/example.liquid'].settings!.push({ + id: 'some-other-setting', + visible_if: '{{ effective_fill == "image" }}', + }); + + const theme = makeTheme(themeData); + theme['blocks/example.liquid'] = ` + {% liquid + assign effective_fill = 'image' + %} + ${theme['blocks/example.liquid']} + `; + + const offenses = await check(theme, [ValidVisibleIf, ValidVisibleIfSettingsSchema]); + expect(offenses).toEqual([]); + }); + it('reports no error for a valid reference to a section schema (simple lookup)', async () => { const themeData = structuredClone(baseThemeData); diff --git a/packages/theme-check-common/src/checks/valid-visible-if/index.ts b/packages/theme-check-common/src/checks/valid-visible-if/index.ts index 531913934..9a33128eb 100644 --- a/packages/theme-check-common/src/checks/valid-visible-if/index.ts +++ b/packages/theme-check-common/src/checks/valid-visible-if/index.ts @@ -1,4 +1,9 @@ -import { type LiquidVariableLookup } from '@shopify/liquid-html-parser'; +import { + NodeTypes, + type LiquidTag, + type LiquidTagAssign, + type LiquidVariableLookup, +} from '@shopify/liquid-html-parser'; import { Severity, SourceCodeType, @@ -39,7 +44,15 @@ export const ValidVisibleIf: LiquidCheckDefinition = { meta: { ...meta, type: SourceCodeType.LiquidHtml }, create(context) { + const assignedVariables: Vars = {}; + return { + async LiquidTag(node) { + if (isLiquidTagAssign(node)) { + assignedVariables[node.markup.name] = true; + } + }, + async LiquidRawTag(node) { if (node.name !== 'schema' || node.body.kind !== 'json') return; @@ -64,7 +77,7 @@ export const ValidVisibleIf: LiquidCheckDefinition = { validSchema.settings.map((setting) => [setting.id, true] as const), ); - const vars: Vars = { settings }; + const vars: Vars = { ...assignedVariables, settings }; if (isSectionSchema(schema)) { vars.section = { settings: currentFileSettings }; } else if (isBlockSchema(schema)) { @@ -123,6 +136,12 @@ export const ValidVisibleIf: LiquidCheckDefinition = { }, }; +function isLiquidTagAssign(node: LiquidTag): node is LiquidTagAssign { + return ( + node.type === NodeTypes.LiquidTag && node.name === 'assign' && typeof node.markup !== 'string' + ); +} + export const ValidVisibleIfSettingsSchema: JSONCheckDefinition = { meta: { ...meta, type: SourceCodeType.JSON }, From dcd9b27f12f1c704db1f8d761b72961885ff7928 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Tue, 15 Sep 2026 08:53:24 +0200 Subject: [PATCH 4/6] Allow private-style Liquid variable names --- .changeset/fix-private-liquid-variables.md | 5 +++ .../src/checks/variable-name/index.spec.ts | 33 +++++++++++++++++++ .../src/checks/variable-name/index.ts | 8 +++-- 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-private-liquid-variables.md diff --git a/.changeset/fix-private-liquid-variables.md b/.changeset/fix-private-liquid-variables.md new file mode 100644 index 000000000..8fc5f506f --- /dev/null +++ b/.changeset/fix-private-liquid-variables.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +Fix Liquid variable-name validation to allow a leading underscore. diff --git a/packages/theme-check-common/src/checks/variable-name/index.spec.ts b/packages/theme-check-common/src/checks/variable-name/index.spec.ts index 9d8321102..28efcabab 100644 --- a/packages/theme-check-common/src/checks/variable-name/index.spec.ts +++ b/packages/theme-check-common/src/checks/variable-name/index.spec.ts @@ -20,6 +20,39 @@ describe('Module: VariableName', () => { expect(offenses).to.be.empty; }); + it('should allow a single leading underscore', async () => { + const sourceCode = ` + {% assign _variable_name = "value" %} + {% capture _captured_value %}value{% endcapture %} + `; + + const offenses = await runLiquidCheck(VariableName, sourceCode); + + expect(offenses).to.be.empty; + }); + + it('should still enforce the naming format after a leading underscore', async () => { + const sourceCode = `{% assign _variableName = "value" %}`; + + const offenses = await runLiquidCheck(VariableName, sourceCode); + + expect(offenses).to.have.length(1); + expect(offenses[0]!.suggest![0].message).to.equal( + "Change variable '_variableName' to '_variable_name'", + ); + }); + + it('should reject more than one leading underscore', async () => { + const sourceCode = `{% assign __variable_name = "value" %}`; + + const offenses = await runLiquidCheck(VariableName, sourceCode); + + expect(offenses).to.have.length(1); + expect(offenses[0]!.suggest![0].message).to.equal( + "Change variable '__variable_name' to '_variable_name'", + ); + }); + it('should provide a suggestion to change the variable naming', async () => { const sourceCode = `{% assign variableName = "value" %}`; diff --git a/packages/theme-check-common/src/checks/variable-name/index.ts b/packages/theme-check-common/src/checks/variable-name/index.ts index f682d908a..681aa5799 100644 --- a/packages/theme-check-common/src/checks/variable-name/index.ts +++ b/packages/theme-check-common/src/checks/variable-name/index.ts @@ -66,10 +66,14 @@ export const VariableName: LiquidCheckDefinition = { } const formatter = formatTypes[context.settings.format as FormatTypes]; - const suggestion = formatter(node.markup.name); + const leadingUnderscore = node.markup.name.startsWith('_') ? '_' : ''; + const name = node.markup.name.slice(leadingUnderscore.length); + const suggestion = leadingUnderscore + formatter(name); return { - valid: collapseNumberSpacing(node.markup.name) === collapseNumberSpacing(suggestion), + valid: + name.length > 0 && + collapseNumberSpacing(node.markup.name) === collapseNumberSpacing(suggestion), suggestion, }; }; From 4d83aa910b6e8a1806418d2000a75ce582b8d950 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Tue, 15 Sep 2026 09:05:38 +0200 Subject: [PATCH 5/6] Follow layout and block references in theme graph JSON templates already connect to their implicit layout, so snippets rendered by that layout are included in the graph. Liquid templates did not establish the same connection, leaving valid layout-rendered snippets incorrectly reported as orphaned. Treat layouts as graph entry points and follow {% block %} dependencies so direct and transitive snippet references are discovered consistently. --- .changeset/fix-layout-snippet-references.md | 5 +++++ .../skeleton/blocks/layout-block.liquid | 1 + .../fixtures/skeleton/layout/theme.liquid | 2 ++ .../skeleton/snippets/block-child.liquid | 1 + .../skeleton/snippets/layout-child.liquid | 1 + .../skeleton/snippets/layout-parent.liquid | 1 + packages/theme-graph/src/graph/build.spec.ts | 22 +++++++++++++++++-- packages/theme-graph/src/graph/build.ts | 6 ++++- packages/theme-graph/src/graph/traverse.ts | 9 ++++++++ 9 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-layout-snippet-references.md create mode 100644 packages/theme-graph/fixtures/skeleton/blocks/layout-block.liquid create mode 100644 packages/theme-graph/fixtures/skeleton/snippets/block-child.liquid create mode 100644 packages/theme-graph/fixtures/skeleton/snippets/layout-child.liquid create mode 100644 packages/theme-graph/fixtures/skeleton/snippets/layout-parent.liquid diff --git a/.changeset/fix-layout-snippet-references.md b/.changeset/fix-layout-snippet-references.md new file mode 100644 index 000000000..7213c7a44 --- /dev/null +++ b/.changeset/fix-layout-snippet-references.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-graph': patch +--- + +Fix orphaned-snippet detection by following layout and `{% block %}` references. diff --git a/packages/theme-graph/fixtures/skeleton/blocks/layout-block.liquid b/packages/theme-graph/fixtures/skeleton/blocks/layout-block.liquid new file mode 100644 index 000000000..dc8cc62d2 --- /dev/null +++ b/packages/theme-graph/fixtures/skeleton/blocks/layout-block.liquid @@ -0,0 +1 @@ +{% render 'block-child' %} diff --git a/packages/theme-graph/fixtures/skeleton/layout/theme.liquid b/packages/theme-graph/fixtures/skeleton/layout/theme.liquid index 49004f777..95307d742 100644 --- a/packages/theme-graph/fixtures/skeleton/layout/theme.liquid +++ b/packages/theme-graph/fixtures/skeleton/layout/theme.liquid @@ -8,6 +8,8 @@ {{ content_for_header }} + {% render 'layout-parent' %} + {% block 'layout-block' %}{% endblock %} {% sections 'header-group' %} {{ content_for_layout }} diff --git a/packages/theme-graph/fixtures/skeleton/snippets/block-child.liquid b/packages/theme-graph/fixtures/skeleton/snippets/block-child.liquid new file mode 100644 index 000000000..d440775e8 --- /dev/null +++ b/packages/theme-graph/fixtures/skeleton/snippets/block-child.liquid @@ -0,0 +1 @@ +Block child diff --git a/packages/theme-graph/fixtures/skeleton/snippets/layout-child.liquid b/packages/theme-graph/fixtures/skeleton/snippets/layout-child.liquid new file mode 100644 index 000000000..b3fd1e7a2 --- /dev/null +++ b/packages/theme-graph/fixtures/skeleton/snippets/layout-child.liquid @@ -0,0 +1 @@ +Layout child diff --git a/packages/theme-graph/fixtures/skeleton/snippets/layout-parent.liquid b/packages/theme-graph/fixtures/skeleton/snippets/layout-parent.liquid new file mode 100644 index 000000000..1e0691022 --- /dev/null +++ b/packages/theme-graph/fixtures/skeleton/snippets/layout-parent.liquid @@ -0,0 +1 @@ +{% render 'layout-child' %} diff --git a/packages/theme-graph/src/graph/build.spec.ts b/packages/theme-graph/src/graph/build.spec.ts index abab11361..821089947 100644 --- a/packages/theme-graph/src/graph/build.spec.ts +++ b/packages/theme-graph/src/graph/build.spec.ts @@ -48,11 +48,12 @@ describe('Module: index', () => { // We're using sections as entry points because the section rendering API can render // any section without it needing a preset or default value in its schema. - it('infers entry points from the templates folder and section files', () => { - expect(graph.entryPoints).toHaveLength(3); + it('infers entry points from templates, layouts, and section files', () => { + expect(graph.entryPoints).toHaveLength(4); expect(graph.entryPoints.map((x) => x.uri)).toEqual( expect.arrayContaining([ p('templates/index.json'), + p('layout/theme.liquid'), p('sections/custom-section.liquid'), p('sections/header.liquid'), ]), @@ -69,6 +70,8 @@ describe('Module: index', () => { expect(deps.map((x) => x.target.uri)).toEqual( expect.arrayContaining([ p('sections/header-group.json'), + p('snippets/layout-parent.liquid'), + p('blocks/layout-block.liquid'), p('assets/theme.js'), p('assets/theme.css'), ]), @@ -83,6 +86,21 @@ describe('Module: index', () => { ); }); + it('follows direct and transitive dependencies from layouts', () => { + expect( + graph.modules[p('snippets/layout-parent.liquid')].references.map((x) => x.source.uri), + ).toContain(p('layout/theme.liquid')); + expect( + graph.modules[p('snippets/layout-child.liquid')].references.map((x) => x.source.uri), + ).toContain(p('snippets/layout-parent.liquid')); + expect( + graph.modules[p('blocks/layout-block.liquid')].references.map((x) => x.source.uri), + ).toContain(p('layout/theme.liquid')); + expect( + graph.modules[p('snippets/block-child.liquid')].references.map((x) => x.source.uri), + ).toContain(p('blocks/layout-block.liquid')); + }); + it("finds templates/index.json's dependencies and references", () => { const indexTemplate = graph.modules[p('templates/index.json')]; assert(indexTemplate); diff --git a/packages/theme-graph/src/graph/build.ts b/packages/theme-graph/src/graph/build.ts index 7f153d24f..594b07557 100644 --- a/packages/theme-graph/src/graph/build.ts +++ b/packages/theme-graph/src/graph/build.ts @@ -21,6 +21,10 @@ export async function buildThemeGraph( // Templates are entry points in the theme graph. const isTemplateFile = uri.startsWith(path.join(rootUri, 'templates')); + // Liquid templates use layout/theme.liquid implicitly, and can select + // other layouts at runtime, so layouts are independently reachable. + const isLayoutFile = uri.startsWith(path.join(rootUri, 'layout')) && uri.endsWith('.liquid'); + // Since any section file can be rendered directly by the Section Rendering API, // we consider all section files as entry points. const isSectionFile = @@ -33,7 +37,7 @@ export async function buildThemeGraph( uri.startsWith(path.join(rootUri, 'blocks')) && uri.endsWith('.liquid'); - return isTemplateFile || isSectionFile || isThemeAppExtensionBlockFile; + return isTemplateFile || isLayoutFile || isSectionFile || isThemeAppExtensionBlockFile; })); const graph: ThemeGraph = { diff --git a/packages/theme-graph/src/graph/traverse.ts b/packages/theme-graph/src/graph/traverse.ts index 6a3c52362..2751b5b13 100644 --- a/packages/theme-graph/src/graph/traverse.ts +++ b/packages/theme-graph/src/graph/traverse.ts @@ -135,6 +135,15 @@ async function traverseLiquidModule( }; }, + // {% block 'block-name' %} + BlockMarkup: (node, ancestors) => { + const tag = ancestors.at(-1)!; + return { + target: getThemeBlockModule(themeGraph, node.name.value), + sourceRange: [tag.position.start, tag.position.end], + }; + }, + // HtmlElement: (node) => { if (node.name.length !== 1) return; From f368042580682a1da8b82d7ce936a8fadcc6db3c Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Mon, 21 Sep 2026 10:28:54 +0200 Subject: [PATCH 6/6] Address theme check review feedback --- .../src/checks/asset-preload/index.spec.ts | 10 ++++++++++ .../src/checks/asset-preload/index.ts | 20 +++++++++---------- .../src/checks/valid-visible-if/index.spec.ts | 17 ++++++++++++++++ .../src/checks/valid-visible-if/index.ts | 9 ++++++++- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/packages/theme-check-common/src/checks/asset-preload/index.spec.ts b/packages/theme-check-common/src/checks/asset-preload/index.spec.ts index e746309be..b2e065a83 100644 --- a/packages/theme-check-common/src/checks/asset-preload/index.spec.ts +++ b/packages/theme-check-common/src/checks/asset-preload/index.spec.ts @@ -55,6 +55,16 @@ describe('Module: AssetPreload', () => { expect(offenses).to.have.lengthOf(0); }); + it('reports image preloading when attribute values contain Liquid', async () => { + const sourceCode = ` + + + `; + + const offenses = await runLiquidCheck(AssetPreload, sourceCode); + expect(offenses).to.have.lengthOf(2); + }); + it('reports general preloading', async () => { const sourceCode = ` diff --git a/packages/theme-check-common/src/checks/asset-preload/index.ts b/packages/theme-check-common/src/checks/asset-preload/index.ts index a3220b633..d9f93bef7 100644 --- a/packages/theme-check-common/src/checks/asset-preload/index.ts +++ b/packages/theme-check-common/src/checks/asset-preload/index.ts @@ -1,6 +1,12 @@ import { NodeTypes, TextNode } from '@shopify/liquid-html-parser'; import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; -import { ValuedHtmlAttribute, isAttr, isNodeOfType, isValuedHtmlAttribute } from '../utils'; +import { + ValuedHtmlAttribute, + hasAttributeValueOf, + isAttr, + isNodeOfType, + isValuedHtmlAttribute, +} from '../utils'; function isPreload(attr: ValuedHtmlAttribute): boolean { return ( @@ -11,16 +17,8 @@ function isPreload(attr: ValuedHtmlAttribute): boolean { function isHighPriorityImagePreload(attributes: ValuedHtmlAttribute[]): boolean { return ( - attributes.some( - (attr) => - isAttr(attr, 'as') && - attr.value.some((node) => node.type === NodeTypes.TextNode && node.value === 'image'), - ) && - attributes.some( - (attr) => - isAttr(attr, 'fetchpriority') && - attr.value.some((node) => node.type === NodeTypes.TextNode && node.value === 'high'), - ) + attributes.some((attr) => isAttr(attr, 'as') && hasAttributeValueOf(attr, 'image')) && + attributes.some((attr) => isAttr(attr, 'fetchpriority') && hasAttributeValueOf(attr, 'high')) ); } diff --git a/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts b/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts index 4555b6f91..b529ee487 100644 --- a/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts +++ b/packages/theme-check-common/src/checks/valid-visible-if/index.spec.ts @@ -117,6 +117,23 @@ describe('Module: ValidVisibleIf', () => { expect(offenses).toEqual([]); }); + it('reports no error for a variable assigned after the schema tag', async () => { + const themeData = structuredClone(baseThemeData); + + themeData['blocks/example.liquid'].settings!.push({ + id: 'some-other-setting', + visible_if: '{{ effective_fill == "image" }}', + }); + + const theme = makeTheme(themeData); + theme['blocks/example.liquid'] += ` + {% assign effective_fill = 'image' %} + `; + + const offenses = await check(theme, [ValidVisibleIf, ValidVisibleIfSettingsSchema]); + expect(offenses).toEqual([]); + }); + it('reports no error for a valid reference to a section schema (simple lookup)', async () => { const themeData = structuredClone(baseThemeData); diff --git a/packages/theme-check-common/src/checks/valid-visible-if/index.ts b/packages/theme-check-common/src/checks/valid-visible-if/index.ts index 9a33128eb..e84eee5b3 100644 --- a/packages/theme-check-common/src/checks/valid-visible-if/index.ts +++ b/packages/theme-check-common/src/checks/valid-visible-if/index.ts @@ -1,5 +1,6 @@ import { NodeTypes, + type LiquidRawTag, type LiquidTag, type LiquidTagAssign, type LiquidVariableLookup, @@ -45,6 +46,7 @@ export const ValidVisibleIf: LiquidCheckDefinition = { create(context) { const assignedVariables: Vars = {}; + let schemaNode: LiquidRawTag | undefined; return { async LiquidTag(node) { @@ -55,6 +57,11 @@ export const ValidVisibleIf: LiquidCheckDefinition = { async LiquidRawTag(node) { if (node.name !== 'schema' || node.body.kind !== 'json') return; + schemaNode = node; + }, + + async onCodePathEnd() { + if (!schemaNode) return; const schema = await getSchema(context); @@ -69,7 +76,7 @@ export const ValidVisibleIf: LiquidCheckDefinition = { return; } - const offset = node.blockStartPosition.end; + const offset = schemaNode.blockStartPosition.end; const settings = Object.fromEntries( (await getGlobalSettings(context)).map((s) => [s, true] as const), );