diff --git a/.changeset/app-block-paths-in-block-tags.md b/.changeset/app-block-paths-in-block-tags.md new file mode 100644 index 000000000..2eca6a13a --- /dev/null +++ b/.changeset/app-block-paths-in-block-tags.md @@ -0,0 +1,13 @@ +--- +'@shopify/liquid-html-parser': minor +'@shopify/theme-check-common': minor +'@shopify/theme-check-node': minor +'@shopify/theme-language-server-common': patch +'@shopify/theme-graph': patch +--- + +Accept canonical app block paths in `{% block %}` tags. + +The parser accepts `shopify://apps//blocks//` as a block type and keeps rejecting malformed app paths. `LiquidSyntaxError` reports arguments and content on app block paths, because app blocks render with the settings their app provides. `block.name` stays allowed. + +App block paths do not produce local theme-file document links or dependencies in the theme graph. diff --git a/packages/liquid-html-parser/src/tags/block.test.ts b/packages/liquid-html-parser/src/tags/block.test.ts index 3f0fea281..b8922ddae 100644 --- a/packages/liquid-html-parser/src/tags/block.test.ts +++ b/packages/liquid-html-parser/src/tags/block.test.ts @@ -229,6 +229,33 @@ describe('blockTag', () => { ); }); + it('parses a canonical app block path', () => { + const path = + 'shopify://apps/example_app/blocks/example-block/00000000-0000-4000-8000-000000000000'; + const result = blockTag.parse('block', parser(`'${path}'`), stubParser); + expect(result.name.value).toBe(path); + }); + + it('rejects malformed app block paths', () => { + const uuid = '00000000-0000-4000-8000-000000000000'; + const invalidPaths = [ + `shopify://apps/example_app/snippets/example-block/${uuid}`, + `shopify://apps//blocks/example-block/${uuid}`, + `shopify://apps/example_app/blocks//${uuid}`, + 'shopify://apps/example_app/blocks/example-block/not-a-uuid', + 'shopify://apps/example_app/blocks/example-block/deadbeef', + `shopify://apps/example_app/blocks/example-block/${uuid}/extra`, + `shopify://apps-evil/example_app/blocks/example-block/${uuid}`, + `shopify://apps/example_app/blocks/example-block/${uuid}\n`, + ]; + + for (const path of invalidPaths) { + expect(() => blockTag.parse('block', parser(`'${path}'`), stubParser)).toThrow( + `in 'block' - '${path}' is not a valid block type`, + ); + } + }); + it('rejects markup without a comma before args', () => { expect(() => blockTag.parse('block', parser("'name' key: value"), stubParser)).toThrow( "Unexpected token in 'block' tag: key", diff --git a/packages/liquid-html-parser/src/tags/block.ts b/packages/liquid-html-parser/src/tags/block.ts index d57ed52e9..e9232d026 100644 --- a/packages/liquid-html-parser/src/tags/block.ts +++ b/packages/liquid-html-parser/src/tags/block.ts @@ -5,6 +5,9 @@ import { NodeTypes } from '../types'; import { TagKind, type TagDefinitionBlock, type Parser } from '../tag-definitions'; const BLOCK_TYPE_REGEX = /^_?[a-zA-Z0-9][\w-]*$/; +// Canonical app block paths include app and block handles followed by a UUID. +const APP_BLOCK_TYPE_REGEX = + /^shopify:\/\/apps\/[A-Za-z0-9][A-Za-z0-9_-]*\/blocks\/[A-Za-z0-9][A-Za-z0-9_-]*\/[0-9A-Fa-f]{8}(?:-[0-9A-Fa-f]{4}){3}-[0-9A-Fa-f]{12}$/; export const blockTag: TagDefinitionBlock = { kind: TagKind.Block, @@ -18,7 +21,7 @@ export const blockTag: TagDefinitionBlock = { if (name.type !== NodeTypes.String) { throw new Error("in 'block' - file name must be a string literal"); } - if (!BLOCK_TYPE_REGEX.test(name.value)) { + if (!BLOCK_TYPE_REGEX.test(name.value) && !APP_BLOCK_TYPE_REGEX.test(name.value)) { throw new Error(`in 'block' - '${name.value}' is not a valid block type`); } diff --git a/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts b/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts index 0858f1b35..55e99a452 100644 --- a/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts +++ b/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts @@ -104,6 +104,14 @@ describe('Module: LiquidHTMLSyntaxError', () => { expect(offenses).to.be.empty; }); + it('should not report canonical inline app block paths', async () => { + const sourceCode = + "{% block 'shopify://apps/example_app/blocks/example-block/00000000-0000-4000-8000-000000000000' %}{% endblock %}"; + + const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode); + expect(offenses).to.be.empty; + }); + it('should highligh the error', async () => { let offenses: Offense[]; let highlights: string[]; diff --git a/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts b/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts index 84c7126f1..2a60c5b51 100644 --- a/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts +++ b/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts @@ -18,6 +18,8 @@ const SYNTAX_ERROR = "Syntax error in 'block' tag"; const BARE_ARRAY_ACCESS = 'Bare bracket access is not allowed in strict2 mode'; const DOTTED_ARGUMENT = "Liquid syntax error: in 'block' - Use plain named arguments, for example: block 'name', heading: value"; +const APP_BLOCK_ARGUMENTS = "Liquid syntax error: in 'block' - app blocks do not accept arguments"; +const APP_BLOCK_CONTENT = "Liquid syntax error: in 'block' - app blocks do not accept content"; const UNCLOSED_BLOCK_PARSER_ERROR = "Attempting to end parsing before LiquidTag 'block' was closed"; const UNCLOSED_BLOCK_IN_LIQUID_PARSER_ERROR = "Unclosed block tag 'block' in {% liquid %} block"; const BLOCK_PARSER_ERROR_MESSAGES = new Set([ @@ -38,11 +40,16 @@ export function blockTagSyntaxError( const markup = node.markup as BlockMarkup; - if (hasInvalidBlockName(markup.name.value)) { - return syntaxProblem( - node.position, - "Liquid syntax error: in 'block' - Valid syntax: block '[file_name]'", - ); + /* + * The parser only accepts theme block names and canonical app block paths + * (shopify://apps//blocks//). App blocks render with the + * settings their app provides, so they take no arguments and no content. + */ + if (isAppBlockPath(markup.name.value)) { + if (markup.args.some((argument) => argument.name !== 'block.name')) { + return syntaxProblem(node.position, APP_BLOCK_ARGUMENTS); + } + if (hasContent(node)) return syntaxProblem(node.position, APP_BLOCK_CONTENT); } /* @@ -100,8 +107,14 @@ function syntaxProblem(position: Position, message: string): Problem child.type !== NodeTypes.TextNode || child.value.trim() !== '', + ); } function isInvalidBlockNameArgument(argument: BlockMarkup['args'][number]): boolean { diff --git a/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts b/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts index cd52be330..db7f5b30f 100644 --- a/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts +++ b/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts @@ -813,6 +813,118 @@ describe('LiquidSyntaxError', () => { }); }); + describe('app block paths', () => { + const APP_BLOCK_PATH = + 'shopify://apps/example_app/blocks/example-block/00000000-0000-4000-8000-000000000000'; + + it.each([ + `{% block '${APP_BLOCK_PATH}' %}{% endblock %}`, + `{% block '${APP_BLOCK_PATH}' %}\n {% endblock %}`, + `{% block '${APP_BLOCK_PATH}', block.name: 'Example' %}{% endblock %}`, + ])('produces no diagnostics for %s', async (template) => { + const offenses = await runLiquidCheck( + LiquidSyntaxError, + template, + 'templates/test.liquid', + NO_DOCSET, + ); + + expect(offenses).toEqual([]); + }); + + it.each([ + [ + `{% block '${APP_BLOCK_PATH}', heading: 'Hello' %}{% endblock %}`, + "Liquid syntax error: in 'block' - app blocks do not accept arguments", + ], + [ + `{% block '${APP_BLOCK_PATH}', block.settings.heading: 'Hello' %}{% endblock %}`, + "Liquid syntax error: in 'block' - app blocks do not accept arguments", + ], + [ + `{% block '${APP_BLOCK_PATH}' %}Hello{% endblock %}`, + "Liquid syntax error: in 'block' - app blocks do not accept content", + ], + [ + `{% block '${APP_BLOCK_PATH}' %}{{ product.title }}{% endblock %}`, + "Liquid syntax error: in 'block' - app blocks do not accept content", + ], + ])('reports %s', async (template, message) => { + const offenses = await runLiquidCheck( + LiquidSyntaxError, + template, + 'templates/test.liquid', + NO_DOCSET, + ); + + expect(offenses).toMatchObject([{ check: 'LiquidSyntaxError', message }]); + }); + + it.each(['1', 'variable'])( + 'rejects a non-string block.name on an app path: %s', + async (value) => { + const offenses = await runLiquidCheck( + LiquidSyntaxError, + `{% block '${APP_BLOCK_PATH}', block.name: ${value} %}{% endblock %}`, + 'templates/test.liquid', + NO_DOCSET, + ); + + expect(offenses).toMatchObject([ + { check: 'LiquidSyntaxError', message: "Syntax error in 'block' tag" }, + ]); + }, + ); + + it('reports malformed app block paths', async () => { + const offenses = await runLiquidCheck( + LiquidSyntaxError, + "{% block 'shopify://apps/example_app/snippets/example-block/00000000-0000-4000-8000-000000000000' %}{% endblock %}", + 'templates/test.liquid', + NO_DOCSET, + ); + + expect(offenses).toMatchObject([ + { check: 'LiquidSyntaxError', message: "Syntax error in 'block' tag" }, + ]); + }); + + it.each([ + `{% block '${APP_BLOCK_PATH}', heading: 'First', heading: 'Second' %}{% endblock %}`, + `{% block '${APP_BLOCK_PATH}', content: 'Explicit' %}Inline{% endblock %}`, + ])('reports only the app syntax error across block checks: %s', async (template) => { + const offenses = await check({ 'templates/test.liquid': template }, [ + LiquidSyntaxError, + MissingBlockArguments, + UnrecognizedBlockArguments, + ValidBlockArgumentTypes, + DuplicateBlockArguments, + ]); + + expect(offenses).toMatchObject([ + { + check: 'LiquidSyntaxError', + message: "Liquid syntax error: in 'block' - app blocks do not accept arguments", + }, + ]); + }); + + it('produces no block parameter diagnostics for app block paths', async () => { + const offenses = await check( + { 'templates/test.liquid': `{% block '${APP_BLOCK_PATH}' %}{% endblock %}` }, + [ + LiquidSyntaxError, + MissingBlockArguments, + UnrecognizedBlockArguments, + ValidBlockArgumentTypes, + DuplicateBlockArguments, + ], + ); + + expect(offenses).toEqual([]); + }); + }); + describe('block caller arguments', () => { it.each([ ['heading', "block.settings.heading: 'Heading'"], diff --git a/packages/theme-graph/src/graph/build.spec.ts b/packages/theme-graph/src/graph/build.spec.ts index 821089947..77702a594 100644 --- a/packages/theme-graph/src/graph/build.spec.ts +++ b/packages/theme-graph/src/graph/build.spec.ts @@ -1,6 +1,7 @@ import { path as pathUtils, SourceCodeType } from '@shopify/theme-check-common'; import { assert, beforeAll, beforeEach, describe, expect, it } from 'vitest'; import { buildThemeGraph } from '../index'; +import { toSourceCode } from '../toSourceCode'; import { Dependencies, JsonModuleKind, LiquidModuleKind, ModuleType, ThemeGraph } from '../types'; import { getDependencies, skeleton, themeAppExtension } from './test-helpers'; @@ -20,6 +21,24 @@ describe('Module: index', () => { expect(graph).toBeDefined(); }); + it.each([ + "{% block 'shopify://apps/example_app/blocks/example-block/00000000-0000-4000-8000-000000000000' %}{% endblock %}", + "{% liquid\n block 'shopify://apps/example_app/blocks/example-block/00000000-0000-4000-8000-000000000000'\n endblock\n %}", + ])('skips app block dependencies while preserving theme blocks: %s', async (appBlock) => { + const graph = await buildThemeGraph(rootUri, { + ...dependencies, + getSourceCode: (uri) => + uri === p('layout/theme.liquid') + ? toSourceCode(uri, `${appBlock}{% block 'text' %}{% endblock %}`) + : dependencies.getSourceCode(uri), + }); + + expect( + graph.modules[p('layout/theme.liquid')].dependencies.map((ref) => ref.target.uri), + ).toEqual([p('blocks/text.liquid')]); + expect(Object.keys(graph.modules).some((uri) => uri.includes('shopify:'))).toBe(false); + }); + describe('with a valid theme graph', () => { let graph: ThemeGraph; diff --git a/packages/theme-graph/src/graph/traverse.ts b/packages/theme-graph/src/graph/traverse.ts index 2751b5b13..8a12e259f 100644 --- a/packages/theme-graph/src/graph/traverse.ts +++ b/packages/theme-graph/src/graph/traverse.ts @@ -137,6 +137,8 @@ async function traverseLiquidModule( // {% block 'block-name' %} BlockMarkup: (node, ancestors) => { + // App blocks are external to the theme, not local file dependencies. + if (node.name.value.startsWith('shopify://apps/')) return; const tag = ancestors.at(-1)!; return { target: getThemeBlockModule(themeGraph, node.name.value), diff --git a/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts b/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts index 235ed1aa7..c0e641924 100644 --- a/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts +++ b/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts @@ -70,6 +70,18 @@ describe('DocumentLinksProvider', () => { } }); + it.each([ + "{% block 'shopify://apps/example_app/blocks/example-block/00000000-0000-4000-8000-000000000000' %}{% endblock %}", + "{% liquid\n block 'shopify://apps/example_app/blocks/example-block/00000000-0000-4000-8000-000000000000'\n endblock\n %}", + ])('should skip app block links while preserving theme block links: %s', async (appBlock) => { + rootUri = 'file:///path/to/project'; + uriString = `${rootUri}/templates/index.liquid`; + documentManager.open(uriString, `${appBlock}{% block 'container' %}{% endblock %}`, 1); + + const result = await documentLinksProvider.documentLinks(uriString); + expect(result.map((link) => link.target)).toEqual([`${rootUri}/blocks/container.liquid`]); + }); + it('should not create a link for the {% partial %} tag', async () => { uriString = 'file:///path/to/liquid-html-document.liquid'; rootUri = 'file:///path/to/project'; diff --git a/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.ts b/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.ts index 911b7de6f..615c03ce3 100644 --- a/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.ts +++ b/packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.ts @@ -66,6 +66,8 @@ function documentLinksVisitor( // {% block 'name' %} if (node.name === NamedTags.block && typeof node.markup !== 'string') { const blockName = node.markup.name; + // App blocks belong to an extension, not the theme's blocks directory. + if (blockName.value.startsWith('shopify://apps/')) return; return DocumentLink.create( range(textDocument, blockName), Utils.resolvePath(root, 'blocks', blockName.value + '.liquid').toString(),