Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions .changeset/app-block-paths-in-block-tags.md
Original file line number Diff line number Diff line change
@@ -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/<app>/blocks/<handle>/<uuid>` 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.
27 changes: 27 additions & 0 deletions packages/liquid-html-parser/src/tags/block.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 4 additions & 1 deletion packages/liquid-html-parser/src/tags/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<BlockMarkup> = {
kind: TagKind.Block,
Expand All @@ -18,7 +21,7 @@ export const blockTag: TagDefinitionBlock<BlockMarkup> = {
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`);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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/<app>/blocks/<handle>/<uuid>). 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);
}

/*
Expand Down Expand Up @@ -100,8 +107,14 @@ function syntaxProblem(position: Position, message: string): Problem<SourceCodeT
return { message, startIndex: position.start, endIndex: position.end };
}

function hasInvalidBlockName(value: string): boolean {
return value.includes('/') || value.includes('.');
function isAppBlockPath(value: string): boolean {
return value.startsWith('shopify://apps/');
}

function hasContent(node: LiquidTag): boolean {
return (node.children ?? []).some(
(child) => child.type !== NodeTypes.TextNode || child.value.trim() !== '',
);
}

function isInvalidBlockNameArgument(argument: BlockMarkup['args'][number]): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'"],
Expand Down
19 changes: 19 additions & 0 deletions packages/theme-graph/src/graph/build.spec.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions packages/theme-graph/src/graph/traverse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Loading