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
9 changes: 9 additions & 0 deletions .changeset/app-block-paths-in-block-tags.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@shopify/liquid-html-parser': minor
'@shopify/theme-check-common': minor
'@shopify/theme-check-node': minor
---

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.
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 @@ -12,6 +12,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 @@ -30,8 +32,16 @@ export function blockTagSyntaxError(node: LiquidTag): string | undefined {

const markup = node.markup as BlockMarkup;

if (hasInvalidBlockName(markup.name.value)) {
return "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 APP_BLOCK_ARGUMENTS;
}
if (hasContent(node)) return APP_BLOCK_CONTENT;
}

/*
Expand Down Expand Up @@ -79,8 +89,14 @@ export function checkBlockParserError(error: Error, context: Context, source: st
});
}

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 @@ -809,6 +809,82 @@ 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('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('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([
"{% block 'card', block.settings.heading: 'Heading' %}{% endblock %}",
Expand Down
Loading