Skip to content

Remove Hybrid Tag #1309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
stephanie-shopify wants to merge 4 commits into
base: main
Choose a base branch
from
+75 −467
Draft

Remove Hybrid Tag #1309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af297ba
Parse section as a standalone tag
stephanie-shopify Sep 29, 2026
dd72662
Treat endsection as an unknown tag instead of a parse error
stephanie-shopify Sep 29, 2026
72e924a
Keep section out of lax tag recovery
stephanie-shopify Sep 29, 2026
334cac7
Limit the endsection exception to endsection
stephanie-shopify Sep 29, 2026
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
6 changes: 6 additions & 0 deletions .changeset/standalone-section-tag.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/liquid-html-parser': patch
'@shopify/theme-check-common': patch
---

Parse `section` as a standalone tag. Removes the hybrid block form, whose forward scan for `{% endsection %}` made files with many `section` tags parse in quadratic time.
32 changes: 6 additions & 26 deletions packages/liquid-html-parser/specs/architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ src/
tokenizer.ts # Source -> Token[] (modal: HTML tag context with =, ", ')
html.ts # HTML element, void, self-closing, raw node, comment, doctype, dangling marker parsing
liquid-blocks.ts # Block tag body parsing, branched block parsing, finalizeBranch
liquid-hybrid.ts # Hybrid tag parsing (section standalone/block detection)
liquid-lines.ts # {% liquid %} line-based parsing, parseLiquidStatement
liquid-raw.ts # Raw tag body parsing, Liquid-in-range parsing
liquid-tags.ts # Tag dispatch: environment lookup, MarkupParser creation, tolerant fallback
Expand Down Expand Up @@ -110,7 +109,6 @@ document/parser.ts (DocumentParser)
| - node-dispatch.ts: token-type switch -> parse function routing
| - liquid-tags.ts: tag dispatch (environment lookup, MarkupParser creation)
| - liquid-blocks.ts: block body + branched block parsing
| - liquid-hybrid.ts: hybrid tag (section) standalone/block detection
| - liquid-raw.ts: raw tag body extraction + Liquid-in-range parsing
| - liquid-variable-output.ts: {{ }} variable output
| - liquid-lines.ts: {% liquid %} line-based parsing
Expand Down Expand Up @@ -156,7 +154,6 @@ The parser calls these utilities while building the AST.
| `document/tree-builder.ts` | Utility functions for the parser: `filterChildren`, `ChildFilterMode`, `mergeAdjacentTextNodes`, `mergeAdjacentTextNodesTrimmed`, `mergeAdjacentTextNodesStripEdges`, `compoundNamesMatch`. Pure functions, zero parser state. | Tokenizing. Parsing. Dispatch. |
| `document/html.ts` | HTML element parsing (open/close matching, unclosed element handling), void elements, self-closing elements, raw HTML nodes (`<script>`, `<style>`, `<svg>`), comments, doctype, dangling marker close. Attribute parsing. Defines `HtmlParserDelegate` interface. | Liquid tag parsing. Expression parsing. |
| `document/liquid-blocks.ts` | Block tag body parsing (`parseBlockBody`), branched block parsing (`parseBranchedBlockBody`), `finalizeBranch`, `peekTagName`, `isBlockTerminator`, `consumeEndTag`. Defines `BlockParserDelegate` interface. | Tag dispatch. Expression parsing. HTML parsing. |
| `document/liquid-hybrid.ts` | Hybrid tag detection: scans forward in token array for matching end tag. Defines `HybridParserDelegate` interface. | Tag dispatch. Expression parsing. |
| `document/liquid-lines.ts` | `{% liquid %}` line-based parsing: `parseLiquidStatement`, line splitting, block nesting within liquid tags. Defines `LineParserDelegate` interface. | Document tokenization. HTML parsing. |
| `document/liquid-raw.ts` | Raw tag body extraction (scan forward for end tag). `parseLiquidInRange` for tags that parse Liquid inside their body. Defines `RawParserDelegate` interface. | Expression parsing. Tag dispatch. |
| `document/liquid-tags.ts` | Tag dispatch: environment lookup, `MarkupParser` creation from envelope, tolerant-mode try/catch fallback to base case. Defines `TagParserDelegate` interface. | Block body parsing. HTML parsing. |
Expand Down Expand Up @@ -495,7 +492,6 @@ enum TagKind {
Block = "block",
Tag = "tag",
Raw = "raw",
Hybrid = "hybrid",
}

type BranchName = "elsif" | "else" | "when";
Expand All @@ -517,16 +513,10 @@ interface TagDefinitionRaw<M = unknown> {
parse(name: string, markup: MarkupParser, parser: Parser): M;
}

interface TagDefinitionHybrid<M = unknown> {
kind: TagKind.Hybrid;
parse(name: string, markup: MarkupParser, parser: Parser): M;
}

type TagDefinition<M = unknown> =
| TagDefinitionBlock<M>
| TagDefinitionTag<M>
| TagDefinitionRaw<M>
| TagDefinitionHybrid<M>;
| TagDefinitionRaw<M>;
```

Dispatch uses switch + `assertNever`:
Expand All @@ -536,7 +526,6 @@ switch (def.kind) {
case TagKind.Block: return parseBlockTag(...);
case TagKind.Tag: return parseTag(...);
case TagKind.Raw: return parseRawTag(...);
case TagKind.Hybrid: return parseHybridTag(...);
default: return assertNever(def);
}
```
Expand Down Expand Up @@ -611,7 +600,7 @@ Defined in frozen `ast.ts`. Produced when a `</tagName>` close tag does not matc

### D1: TagDefinition as discriminated union on `kind`

Tags define behavior via `kind: TagKind.Block | TagKind.Tag | TagKind.Raw | TagKind.Hybrid` with generic `Markup` parameter. Block tags declare `branches: BranchName[]`. The `kind` discriminant drives exhaustive dispatch (see Section 4, TagDefinition).
Tags define behavior via `kind: TagKind.Block | TagKind.Tag | TagKind.Raw` with generic `Markup` parameter. Block tags declare `branches: BranchName[]`. The `kind` discriminant drives exhaustive dispatch (see Section 4, TagDefinition).

### D2: Internal BinaryExpression with adapter to frozen types

Expand Down Expand Up @@ -655,9 +644,9 @@ See Section 2 for domain table and import rules.

Tokenizer produces flat `[LiquidTagOpen, Text, LiquidTagClose, ...]`. Parser matches pairs by consuming forward. Pairing is the parser's job.

### D12: Section hybrid detection via lookahead
### D12: `section` is a standalone tag

When parser encounters `{% section %}`, it scans forward in the token array for `{% endsection %}`. The array is materialized, so scanning is a cheap index walk.
`section` is a plain `TagKind.Tag`. Ruby Liquid has no block form, so `{% endsection %}` is a structural error (Theme Check reports it as `Unknown tag 'endsection'`). An earlier hybrid design scanned forward for `{% endsection %}` on every `section`, which made files with many sections parse in quadratic time.

### D13: HTML parsing in the document domain

Expand Down Expand Up @@ -768,13 +757,9 @@ liquid tag parse(name, markupParser, parser):

**Inline comments:** Lines starting with `#` are intercepted before environment lookup — `#` is not a valid identifier start, so normal tag name extraction (`content.split(/\s/)[0]`) cannot handle it. The parser checks `content.startsWith('#')` and short-circuits to produce `LiquidTag { name: '#', markup: 'rest of line' }` directly.

### 6.4 Section Hybrid Tag

`section` is the only hybrid tag: standalone (`{% section 'name' %}`) or block (`{% section 'name' %}...{% endsection %}`).
### 6.4 Section Tag

**Detection:** When parser encounters `{% section %}`, it scans forward in the token array for `{% endsection %}`, respecting nesting. If found, parse as block. Otherwise, standalone.

**Markup for both forms:** `SectionMarkup { name: LiquidString, args: LiquidNamedArgument[] }`. Same regardless of form. The difference is whether `children` is populated and `blockEndPosition` exists.
`section` is standalone only (`{% section 'name' %}`), producing `SectionMarkup { name: LiquidString, args: LiquidNamedArgument[] }`. See D12.

### 6.5 Tolerant Mode vs Completion Mode

Expand Down Expand Up @@ -870,11 +855,6 @@ Parser sees LiquidTagOpen token
| If tagName == 'doc': pass body to liquid-doc/parser
| makeLiquidRawTag(envelope, body, endPos, endWs)
|
+-- TagKind.Hybrid ->
| Scan forward for matching {% endtagname %}
| If found: parse as block
| If not found: parse as standalone
|
+-- default -> assertNever(def)
```

Expand Down
1 change: 0 additions & 1 deletion packages/liquid-html-parser/specs/recursive-descent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ Standalone tags (no close tag) have `delimiterWhitespaceStart`/`delimiterWhitesp
| **Branching** (delimiters inside blocks) | `elsif`, `else`, `when` | Creates `LiquidBranch` nodes inside parent block. |
| **Raw** (body not parsed) | `raw`, `comment`, `doc`, `javascript`, `schema`, `style`, `stylesheet` | Body is raw string until `{% endtagname %}`. |
| **Standalone** (no close tag) | `echo`, `assign`, `render`, `include`, `increment`, `decrement`, `cycle`, `layout`, `section`, `sections`, `content_for`, `break`, `continue`, `liquid` | Self-contained, no children. |
| **Hybrid** | `section` can be both standalone AND block form (`{% section 'name' %}...{% endsection %}`) | Check for presence of `{% endsection %}`. |

**Position fields on block tags and branches:**

Expand Down
52 changes: 16 additions & 36 deletions packages/liquid-html-parser/src/ast.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,50 +1105,30 @@ describe('Unit: Stage 2 (AST)', () => {
}
});

it('should parse section hybrid (block form) with endsection', () => {
it('should treat section as a standalone tag and endsection as an unknown tag', () => {
for (const { toAST, expectPath } of testCases) {
const source = `{% section 'foo' %}content{% endsection %}`;
ast = toAST(source);
expectPath(ast, 'children.0.type').to.eql('LiquidTag');
ast = toAST(`{% section 'foo' %}content{% endsection %}`);
expectPath(ast, 'children.0.name').to.eql('section');
expectPath(ast, 'children.0.markup.name.value').to.eql('foo');
expectPath(ast, 'children.0.children.0.type').to.eql('TextNode');
expectPath(ast, 'children.0.children.0.value').to.eql('content');

// blockEndPosition should span the {% endsection %} tag exactly
expectPath(ast, 'children.0.blockEndPosition.start').to.eql(
source.indexOf('{% endsection %}'),
);
expectPath(ast, 'children.0.blockEndPosition.end').to.eql(source.length);

// section node position.end should match endsection's end
expectPath(ast, 'children.0.position.end').to.eql(source.length);
expectPath(ast, 'children.0.children').to.eql(undefined);
expectPath(ast, 'children.1.value').to.eql('content');
expectPath(ast, 'children.2.name').to.eql('endsection');

expectPath(ast, 'children.0.delimiterWhitespaceStart').to.eql('');
expectPath(ast, 'children.0.delimiterWhitespaceEnd').to.eql('');
}
});
ast = toAST(`{% endsection %}`);
expectPath(ast, 'children.0.name').to.eql('endsection');

it('should capture whitespace trimming on endsection', () => {
for (const { toAST, expectPath } of testCases) {
const source = `{% section 'foo' %}content{%- endsection -%}`;
ast = toAST(source);
expectPath(ast, 'children.0.name').to.eql('section');
expectPath(ast, 'children.0.delimiterWhitespaceStart').to.eql('-');
expectPath(ast, 'children.0.delimiterWhitespaceEnd').to.eql('-');
expectPath(ast, 'children.0.blockEndPosition.start').to.eql(
source.indexOf('{%- endsection -%}'),
);
expectPath(ast, 'children.0.blockEndPosition.end').to.eql(source.length);
expectPath(ast, 'children.0.position.end').to.eql(source.length);
ast = toAST(`{% if a %}{% endsection %}{% endif %}`);
expectPath(ast, 'children.0.name').to.eql('if');
expectPath(ast, 'children.0.children.0.children.0.name').to.eql('endsection');
}
});

it('should throw on orphaned endsection', () => {

@stephanie-shopify stephanie-shopify Sep 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this here? why does removing hybrid tag change it?

@stephanie-shopify stephanie-shopify Sep 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On main, any end tag for a registered tag with no opener is a parse error ({% endrender %}, {% endif %}, …). section is registered, so a lone {% endsection %} threw too. With the hybrid form, a paired endsection was legitimate, and this test pinned that difference: paired parses, orphan throws.

Once section isn't a block, every endsection is effectively an orphan. Left alone, the legacy {% section %}…{% endsection %} form would go from "Unknown tag, other checks still run" to "whole file fails to parse" (theme-check skips every other check on files that fail to parse). So I exempted endsection from that throw, which is what flips this test. Other tags' end-tag errors are unchanged, and there's a regression test for that.

This, the checker move and the new export all follow from removing the block form. They also cause a few remaining differences on already-invalid Liquid, which the tophat caught: different messages for a lone endsection, one in {% liquid %} and one inside HTML, and Prettier output changes for those. <p>{% section 'x' %}y{% endsection %}</p> gets mangled.

Alternative with zero behavior change: keep section hybrid and replace the per-tag forward scan with one pass that pairs each section with its endsection (like matching parentheses), per document and per {% liquid %} block. That's still linear in the worst case, and theme-check, Prettier and all existing tests stay as on main. Removing the block form could then be a separate follow-up where the behavior change is the explicit point. I'm leaning that way. I'll check with CP, since removing hybrid was their suggestion.

@stephanie-shopify stephanie-shopify Sep 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative with zero behavior change: keep section hybrid and replace the per-tag forward scan with one pass that pairs each section with its endsection

dont want to do this. defeats purpose of removing hybrid section tag.

it('should still throw on stray end tags for other registered tags', () => {
for (const { toAST } of testCases) {
expect(() => {
toAST(`{% endsection %}`);
}).to.throw(/without a matching/);
expect(() => toAST(`{% endrender %}`)).to.throw(/without a matching 'render'/);
expect(() => toAST(`{% endsections %}`)).to.throw(/without a matching 'sections'/);
expect(() => toAST(`{% if a %}{% endrender %}{% endif %}`)).to.throw(
/before LiquidTag 'if' was closed/,
);
}
});

Expand Down
5 changes: 3 additions & 2 deletions packages/liquid-html-parser/src/document/liquid-blocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type {
LiquidBranchNamed,
AttributeNode,
} from '../ast';
import { isStructuralEndTag } from '../environment';
import type { TagDefinitionBlock, BranchName, Parser } from '../environment';
import { LiquidHTMLASTParsingError } from '../errors';
import { assertNever } from '../utils';
Expand Down Expand Up @@ -126,7 +127,7 @@ export function parseBlockBody(

if (tagName && tagName.startsWith('end')) {
const innerName = tagName.slice(3);
if (parser.blockEnv.tagForName(innerName)) {
if (isStructuralEndTag(innerName, parser.blockEnv.tagForName(innerName))) {
throw new LiquidHTMLASTParsingError(
`Attempting to close LiquidTag '${innerName}' before LiquidTag '${parentName}' was closed`,
parser.getSource(),
Expand Down Expand Up @@ -252,7 +253,7 @@ export function parseBranchedBody(

if (tagName && tagName.startsWith('end')) {
const innerName = tagName.slice(3);
if (parser.blockEnv.tagForName(innerName)) {
if (isStructuralEndTag(innerName, parser.blockEnv.tagForName(innerName))) {
throw new LiquidHTMLASTParsingError(
`Attempting to close LiquidTag '${innerName}' before LiquidTag '${parentName}' was closed`,
parser.getSource(),
Expand Down
Loading
Loading