From c8099ba8b70770548ea0dff5f6c469e05c86a335 Mon Sep 17 00:00:00 2001 From: Mathieu Perreault Date: Thu, 29 Oct 2026 10:59:04 -0400 Subject: [PATCH 1/3] Avoid repeated token scans in liquid-html-parser Preserve the first-token-at-or-after semantics for forward and backward seeks while using sorted token offsets for bounded lookup work. Co-authored-by: River --- .changeset/linear-html-comment-seek.md | 5 + .../src/document/base.test.ts | 94 +++++++++++++++++++ .../liquid-html-parser/src/document/base.ts | 17 +++- 3 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 .changeset/linear-html-comment-seek.md create mode 100644 packages/liquid-html-parser/src/document/base.test.ts diff --git a/.changeset/linear-html-comment-seek.md b/.changeset/linear-html-comment-seek.md new file mode 100644 index 000000000..8ac0cf2f3 --- /dev/null +++ b/.changeset/linear-html-comment-seek.md @@ -0,0 +1,5 @@ +--- +'@shopify/liquid-html-parser': patch +--- + +Find the parser's position after each HTML comment and `'; + const ast = toLiquidHtmlAST(element.repeat(n)); + expect(ast.children).toHaveLength(n); + expect(ast.children.every((child) => child.type === 'HtmlRawNode')).toBe(true); + expect(ast.children[n - 1].position).toEqual({ + start: (n - 1) * element.length, + end: n * element.length, + }); + }); +}); diff --git a/packages/liquid-html-parser/src/document/base.ts b/packages/liquid-html-parser/src/document/base.ts index 6d9125017..c93662b9a 100644 --- a/packages/liquid-html-parser/src/document/base.ts +++ b/packages/liquid-html-parser/src/document/base.ts @@ -90,11 +90,22 @@ export class ParserBase { * that consumes the element's close tag as text while seeking `{% endif %}`). * Callers that know a token boundary by offset should re-seek here rather than * trusting wherever a nested parse left the cursor. + * + * Token starts are non-decreasing (the tokenizer emits tokens in source + * order and `resliceTokensFrom` keeps that order), so a binary search finds + * the same token a scan from index 0 would, in O(log n), whichever side of + * the cursor it is on. This runs once per HTML comment and raw element, so + * a linear scan here made parsing quadratic in the number of those nodes. */ seekToSourceOffset(sourceOffset: number): void { - let i = 0; - while (i < this.tokens.length && this.tokens[i].start < sourceOffset) i++; - this.p = i; + let lo = 0; + let hi = this.tokens.length; + while (lo < hi) { + const mid = (lo + hi) >>> 1; + if (this.tokens[mid].start < sourceOffset) lo = mid + 1; + else hi = mid; + } + this.p = lo; } /** From 8ebb6aaa515986b1e9b33993c0b0d027e7341686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Thu, 29 Oct 2026 12:12:58 -0400 Subject: [PATCH 2/3] Binary-search the token reslice after raw Liquid tags resliceTokensFrom runs once per raw Liquid tag, like {% raw %} or {% comment %}, and still scanned every token: once for a token straddling the end tag, and again, when none did, for the first token after it. So parsing stayed quadratic in the number of those tags: 30,000 {% comment %}x{% endcomment %} took 6 s, and now take 34 ms. Tokens are in source order and don't overlap, so the only token that can straddle the end tag is the first one ending past it, and a binary search finds it. When none straddles, the reslice seeks, which is the binary search from the previous commit. Both are one helper now. The two tests that parsed 2,000 comments or raw elements become a spec case with four of them side by side. They passed before the fix too, so what they checked was the tree, and the spec checks all of it, in every parser that runs the specs. Co-Authored-By: Claude Opus 5.5 --- .changeset/linear-html-comment-seek.md | 2 +- .../src/document/base.test.ts | 31 +++++-------- .../liquid-html-parser/src/document/base.ts | 31 ++++++++----- specs/parser/html.yml | 44 +++++++++++++++++++ 4 files changed, 76 insertions(+), 32 deletions(-) diff --git a/.changeset/linear-html-comment-seek.md b/.changeset/linear-html-comment-seek.md index 8ac0cf2f3..b9ecd924d 100644 --- a/.changeset/linear-html-comment-seek.md +++ b/.changeset/linear-html-comment-seek.md @@ -2,4 +2,4 @@ '@shopify/liquid-html-parser': patch --- -Find the parser's position after each HTML comment and `'; - const ast = toLiquidHtmlAST(element.repeat(n)); - expect(ast.children).toHaveLength(n); - expect(ast.children.every((child) => child.type === 'HtmlRawNode')).toBe(true); - expect(ast.children[n - 1].position).toEqual({ - start: (n - 1) * element.length, - end: n * element.length, - }); +describe('Unit: ParserBase#resliceTokensFrom', () => { + it('does not rescan the tokens on successive reslices that need no re-tokenize', () => { + // parseLiquidRaw reslices once per raw Liquid tag ({% raw %}, {% comment %}, ...). + const n = 4096; + const { tokens, reads } = countingTokens(n); + const parser = new ParserBase(tokens, ' '.repeat(n)); + for (let offset = 1; offset <= n; offset++) { + parser.resliceTokensFrom(offset); + expect(parser.getPosition()).toBe(offset); + } + expect(reads.count).toBeLessThan(n * 32); }); }); diff --git a/packages/liquid-html-parser/src/document/base.ts b/packages/liquid-html-parser/src/document/base.ts index c93662b9a..695d6b7ef 100644 --- a/packages/liquid-html-parser/src/document/base.ts +++ b/packages/liquid-html-parser/src/document/base.ts @@ -94,18 +94,28 @@ export class ParserBase { * Token starts are non-decreasing (the tokenizer emits tokens in source * order and `resliceTokensFrom` keeps that order), so a binary search finds * the same token a scan from index 0 would, in O(log n), whichever side of - * the cursor it is on. This runs once per HTML comment and raw element, so - * a linear scan here made parsing quadratic in the number of those nodes. + * the cursor it is on. This runs once per HTML comment, raw element and raw + * Liquid tag, so a linear scan here made parsing quadratic in the number of + * those nodes. */ seekToSourceOffset(sourceOffset: number): void { + this.p = this.countTokensWhile((token) => token.start < sourceOffset); + } + + /** + * How many tokens, from the first, pass `test`, by binary search. Tokens are + * in source order and don't overlap, so a test like "starts before" or "ends + * at or before" an offset passes for a run of leading tokens and no others. + */ + private countTokensWhile(test: (token: Token) => boolean): number { let lo = 0; let hi = this.tokens.length; while (lo < hi) { const mid = (lo + hi) >>> 1; - if (this.tokens[mid].start < sourceOffset) lo = mid + 1; + if (test(this.tokens[mid])) lo = mid + 1; else hi = mid; } - this.p = lo; + return lo; } /** @@ -176,12 +186,13 @@ export class ParserBase { } resliceTokensFrom(sourceOffset: number): void { - const straddles = this.tokens.some( - (token) => token.start < sourceOffset && token.end > sourceOffset, - ); + // Tokens don't overlap, so only the first one ending past the boundary can + // straddle it. + const before = this.countTokensWhile((token) => token.end <= sourceOffset); + const straddles = before < this.tokens.length && this.tokens[before].start < sourceOffset; if (straddles) { - const prefix = this.tokens.filter((token) => token.end <= sourceOffset); + const prefix = this.tokens.slice(0, before); // If the boundary lies inside a quoted HTML attribute value (the raw tag // was dispatched from `parseQuotedAttributeValue`), the suffix must be // re-tokenized in QuotedValue mode so the attribute's closing quote and @@ -214,8 +225,6 @@ export class ParserBase { // past the raw end tag is the only thing the reslice needed to do here. // (The trailing EndOfInput token has `start === source.length`, so it is // never skipped past, keeping `this.p` in bounds.) - let i = 0; - while (i < this.tokens.length && this.tokens[i].start < sourceOffset) i++; - this.p = i; + this.seekToSourceOffset(sourceOffset); } } diff --git a/specs/parser/html.yml b/specs/parser/html.yml index 8e5047ca9..7df09c1f6 100644 --- a/specs/parser/html.yml +++ b/specs/parser/html.yml @@ -4928,3 +4928,47 @@ cases: value: " b" position: { start: [0, 34], text: " b" } children: [] + - name: should parse HTML comments and raw elements side by side + source: + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: } + children: + - type: HtmlComment + body: a + position: { start: [0, 0], text: } + - type: HtmlRawNode + name: script + blockStartPosition: { start: [0, 8], text: } + position: { start: [0, 8], text: } + attributes: [] + body: + type: RawMarkup + kind: javascript + value: b + position: { start: [0, 16], text: b } + nodes: + - type: TextNode + value: b + position: { start: [0, 16], text: b } + - type: HtmlComment + body: c + position: { start: [0, 26], text: } + - type: HtmlRawNode + name: style + blockStartPosition: { start: [0, 34], text: } + position: { start: [0, 34], text: } + attributes: [] + body: + type: RawMarkup + kind: css + value: d + position: { start: [0, 41], text: d } + nodes: + - type: TextNode + value: d + position: { start: [0, 41], text: d } From 5877ab7a1a35e9ab7746575bb5e8e9e897d135b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Thu, 29 Oct 2026 13:02:11 -0400 Subject: [PATCH 3/3] Re-tokenize after raw Liquid tags only as far as read A stray {{ or {% in a raw tag's body, as in {% raw %}{{ x{% endraw %}, leaves the tokenizer inside Liquid output at the end tag. The token there runs past it, so resliceTokensFrom re-tokenizes from the end tag on, and it did so through the end of the file, once per such tag. 3,000 of them took 1.5 to 22 s, depending on what follows, and now take 9 to 59 ms. The tokenizer now produces tokens as the parser reads them, and the next straddling tag replaces them again, so each part of the file is tokenized about once. For that: - createTokenizer restarts at an offset and fills on demand, and tokenize is that, run to the end; - inside Liquid output, the tokenizer jumps to the next }} with indexOf, and remembers where it found one, since restarts can land inside the same unclosed {{ many times; - the HTML context a reslice resumes in, an open quoted attribute or tag, is folded over the kept tokens incrementally, instead of from the first token each time. Callers that read tokens by index go through hasToken and tokenAt, which tokenize up to the index, and tokenCount is gone. The new test counts the tokenizer's probes of the source across 200 straddling reslices: about 4,000 now, 2.8 million before. Whole files parse no slower: Dawn and Horizon take 0.96 to 1.0 times as long. Across 350,000 generated documents with straddling raw tags, 200,000 fuzzed ones and the 333 theme files, the tokens, trees and errors from all four parser entry points are the same as before. Co-Authored-By: Claude Opus 5.5 --- .changeset/linear-html-comment-seek.md | 2 +- .../src/document/base.test.ts | 30 +- .../liquid-html-parser/src/document/base.ts | 150 ++++---- .../liquid-html-parser/src/document/html.ts | 10 +- .../src/document/liquid-raw.ts | 8 +- .../src/document/tokenizer.ts | 348 ++++++++++-------- 6 files changed, 324 insertions(+), 224 deletions(-) diff --git a/.changeset/linear-html-comment-seek.md b/.changeset/linear-html-comment-seek.md index b9ecd924d..46acdf023 100644 --- a/.changeset/linear-html-comment-seek.md +++ b/.changeset/linear-html-comment-seek.md @@ -2,4 +2,4 @@ '@shopify/liquid-html-parser': patch --- -Find the parser's position after each HTML comment, `` (NBSP) is body text, not a close. let end = i + 1; - while (end < tokenCount && parser.tokenAt(end).type === TokenType.Text) end++; + while (parser.hasToken(end) && parser.tokenAt(end).type === TokenType.Text) end++; if (end === i + 1) continue; const text = source.slice(token.end, parser.tokenAt(end - 1).end); if (text.trim().toLowerCase() === lowerName) { diff --git a/packages/liquid-html-parser/src/document/liquid-raw.ts b/packages/liquid-html-parser/src/document/liquid-raw.ts index 56690aad5..5e8d67d1e 100644 --- a/packages/liquid-html-parser/src/document/liquid-raw.ts +++ b/packages/liquid-html-parser/src/document/liquid-raw.ts @@ -93,7 +93,7 @@ export function parseLiquidInRange( // token at each step (and re-checking against `bodyEnd`) keeps the walk // correct across that mutation; a cached end index would point at a stale slot. let i = parser.getPosition(); - while (i < parser.tokenCount() && parser.tokenAt(i).start < bodyEnd) { + while (parser.hasToken(i) && parser.tokenAt(i).start < bodyEnd) { const token = parser.tokenAt(i); if ( @@ -112,12 +112,12 @@ export function parseLiquidInRange( ? TokenType.LiquidVariableOutputClose : TokenType.LiquidTagClose; let j = i + 1; - while (j < parser.tokenCount() && parser.tokenAt(j).type !== closeType) j++; - const closesWithinBody = j < parser.tokenCount() && parser.tokenAt(j).end <= bodyEnd; + while (parser.hasToken(j) && parser.tokenAt(j).type !== closeType) j++; + const closesWithinBody = parser.hasToken(j) && parser.tokenAt(j).end <= bodyEnd; if (!closesWithinBody) { nodes.push(makeTextNode(source.slice(token.start, bodyEnd), token.start, bodyEnd, source)); - while (i < parser.tokenCount() && parser.tokenAt(i).start < bodyEnd) i++; + while (parser.hasToken(i) && parser.tokenAt(i).start < bodyEnd) i++; break; } diff --git a/packages/liquid-html-parser/src/document/tokenizer.ts b/packages/liquid-html-parser/src/document/tokenizer.ts index 229c6ab9b..2d9345c0f 100644 --- a/packages/liquid-html-parser/src/document/tokenizer.ts +++ b/packages/liquid-html-parser/src/document/tokenizer.ts @@ -63,6 +63,31 @@ export interface TokenizeOptions { export function tokenize(source: string, options: TokenizeOptions = {}): Token[] { const tokens: Token[] = []; + const tokenizer = createTokenizer(source, tokens); + tokenizer.restart(0, options); + tokenizer.fill(Infinity); + return tokens; +} + +/** + * `tokenize`, run on demand. `ParserBase.resliceTokensFrom` uses it to + * re-tokenize the rest of a document only as far as the parser reads. + */ +export interface Tokenizer { + /** + * Start over at `start`, as if `source` began there; tokens keep their + * offsets in `source`. + */ + restart(start: number, options?: TokenizeOptions): void; + + /** + * Add tokens until there are more than `index` of them, or until the input + * runs out, which adds `EndOfInput`. Returns whether input remains. + */ + fill(index: number): boolean; +} + +export function createTokenizer(source: string, tokens: Token[]): Tokenizer { const modeStack: Mode[] = []; let mode = Mode.Default as Mode; let pos = 0; @@ -70,21 +95,12 @@ export function tokenize(source: string, options: TokenizeOptions = {}): Token[] // Whether the pending Text run is whitespace. Only HtmlTag mode reads it. let textIsSpace = false; let quoteChar = ''; - - // Resume inside a quoted attribute value when reslicing a suffix that begins - // mid-attribute (e.g. `...{% endraw %}">` straddled by a stray `{{`). The - // enclosing HtmlTag mode is kept beneath QuotedValue so popping on the - // closing quote correctly emits the attribute close and trailing `>`. - if (options.insideQuotedAttribute) { - quoteChar = options.insideQuotedAttribute; - modeStack.push(Mode.HtmlTag); - mode = Mode.QuotedValue; - } else if (options.insideHtmlTag) { - // Resume inside the attribute list of an open tag. The empty mode stack - // means the closing `>` pops back to `Default`, so the rest of the suffix - // (`>ok`) tokenizes as normal document content. - mode = Mode.HtmlTag; - } + let ended = false; + // The last search for `}}`: where it started, and where it found one (-1 for + // nowhere). Restarts after raw tags can land inside the same unclosed `{{` + // again and again, and this keeps each from searching the rest anew. + let closeSearchFrom = Infinity; + let closeFound = -1; function ch(offset: number): string { const i = pos + offset; @@ -121,17 +137,53 @@ export function tokenize(source: string, options: TokenizeOptions = {}): Token[] if (textStart === -1) textStart = pos; } - // YAML frontmatter: only at position 0 - if (!options.skipFrontmatter && (match('---\n') || match('---\r\n'))) { - const searchStart = source.indexOf('\n', 0) + 1; - const closeIdx = source.indexOf('\n---', searchStart); - if (closeIdx !== -1) { - let end = closeIdx + 4; // after \n--- - // Include trailing newline/CRLF - if (end < source.length && source[end] === '\r') end++; - if (end < source.length && source[end] === '\n') end++; - tokens.push({ type: TokenType.YamlFrontmatter, start: 0, end }); - pos = end; + /** + * Where the `-}}` or `}}` that closes Liquid output at `pos` begins, or the + * end of the input. Neither begins at `pos` itself. + */ + function nextOutputClose(): number { + if (pos < closeSearchFrom || (closeFound !== -1 && pos > closeFound)) { + closeSearchFrom = pos; + closeFound = source.indexOf('}}', pos); + } + if (closeFound === -1) return source.length; + return source[closeFound - 1] === '-' ? closeFound - 1 : closeFound; + } + + function restart(start: number, options: TokenizeOptions = {}) { + modeStack.length = 0; + mode = Mode.Default; + pos = start; + textStart = -1; + ended = false; + + // Resume inside a quoted attribute value when reslicing a suffix that begins + // mid-attribute (e.g. `...{% endraw %}">` straddled by a stray `{{`). The + // enclosing HtmlTag mode is kept beneath QuotedValue so popping on the + // closing quote correctly emits the attribute close and trailing `>`. + if (options.insideQuotedAttribute) { + quoteChar = options.insideQuotedAttribute; + modeStack.push(Mode.HtmlTag); + mode = Mode.QuotedValue; + } else if (options.insideHtmlTag) { + // Resume inside the attribute list of an open tag. The empty mode stack + // means the closing `>` pops back to `Default`, so the rest of the suffix + // (`>ok`) tokenizes as normal document content. + mode = Mode.HtmlTag; + } + + // YAML frontmatter: only at position 0 + if (!options.skipFrontmatter && (match('---\n') || match('---\r\n'))) { + const searchStart = source.indexOf('\n', pos) + 1; + const closeIdx = source.indexOf('\n---', searchStart); + if (closeIdx !== -1) { + let end = closeIdx + 4; // after \n--- + // Include trailing newline/CRLF + if (end < source.length && source[end] === '\r') end++; + if (end < source.length && source[end] === '\n') end++; + tokens.push({ type: TokenType.YamlFrontmatter, start: pos, end }); + pos = end; + } } } @@ -160,153 +212,161 @@ export function tokenize(source: string, options: TokenizeOptions = {}): Token[] return false; } - while (pos < source.length) { - switch (mode) { - case Mode.LiquidTag: { - if (match('-%}')) { - emit(TokenType.LiquidTagClose, 3); - popMode(); - } else if (match('%}')) { - emit(TokenType.LiquidTagClose, 2); - popMode(); - } else { - startText(); - pos++; - } - break; - } - - case Mode.LiquidVariableOutput: { - if (match('-}}')) { - emit(TokenType.LiquidVariableOutputClose, 3); - popMode(); - } else if (match('}}')) { - emit(TokenType.LiquidVariableOutputClose, 2); - popMode(); - } else { - startText(); - pos++; + function fill(index: number): boolean { + while (pos < source.length && tokens.length <= index) { + switch (mode) { + case Mode.LiquidTag: { + if (match('-%}')) { + emit(TokenType.LiquidTagClose, 3); + popMode(); + } else if (match('%}')) { + emit(TokenType.LiquidTagClose, 2); + popMode(); + } else { + startText(); + pos++; + } + break; } - break; - } - - case Mode.Default: { - if (scanLiquidOpen()) continue; - if (match('')) { - emit(TokenType.HtmlCommentClose, 3); - continue; - } + case Mode.Default: { + if (scanLiquidOpen()) continue; - if (match('')) { + emit(TokenType.HtmlCommentClose, 3); continue; } - } - if (ch(0) === '<') { - const after = ch(1); - if (/[a-zA-Z]/.test(after) || after === '{') { - emit(TokenType.HtmlTagOpen, 1); + if (match('')) { - emit(TokenType.HtmlSelfClose, 2); - popMode(); - continue; + startText(); + pos++; + break; } - if (ch(0) === '>') { - emit(TokenType.HtmlTagClose, 1); - popMode(); - continue; - } + case Mode.HtmlTag: { + if (scanLiquidOpen()) continue; - if (ch(0) === '=') { - emit(TokenType.HtmlEquals, 1); - continue; - } + if (match('/>')) { + emit(TokenType.HtmlSelfClose, 2); + popMode(); + continue; + } - // Accept straight quotes and curly (smart) quotes as attribute-value - // openers. Curly quotes get normalized to straight quotes downstream; - // recognizing them here (HTML scope only — Liquid tokenization is - // untouched) stops the value from splitting at interior spaces. - if ( - ch(0) === '"' || - ch(0) === "'" || - ch(0) === '“' || - ch(0) === '”' || - ch(0) === '‘' || - ch(0) === '’' - ) { - quoteChar = ch(0); - emit(TokenType.HtmlQuoteOpen, 1); - pushMode(Mode.QuotedValue); - continue; + if (ch(0) === '>') { + emit(TokenType.HtmlTagClose, 1); + popMode(); + continue; + } + + if (ch(0) === '=') { + emit(TokenType.HtmlEquals, 1); + continue; + } + + // Accept straight quotes and curly (smart) quotes as attribute-value + // openers. Curly quotes get normalized to straight quotes downstream; + // recognizing them here (HTML scope only — Liquid tokenization is + // untouched) stops the value from splitting at interior spaces. + if ( + ch(0) === '"' || + ch(0) === "'" || + ch(0) === '“' || + ch(0) === '”' || + ch(0) === '‘' || + ch(0) === '’' + ) { + quoteChar = ch(0); + emit(TokenType.HtmlQuoteOpen, 1); + pushMode(Mode.QuotedValue); + continue; + } + + // A run of whitespace inside a tag is a Text token of its own, never + // part of a name or an unquoted value. It plays the part of ohm's + // `space`, which the grammar's syntactic attribute rules skipped + // implicitly (`attr = "v"` is one attribute). + const isSpace = /\s/.test(ch(0)); + if (textStart !== -1 && isSpace !== textIsSpace) flushText(); + if (textStart === -1) textIsSpace = isSpace; + startText(); + pos++; + break; } - // A run of whitespace inside a tag is a Text token of its own, never - // part of a name or an unquoted value. It plays the part of ohm's - // `space`, which the grammar's syntactic attribute rules skipped - // implicitly (`attr = "v"` is one attribute). - const isSpace = /\s/.test(ch(0)); - if (textStart !== -1 && isSpace !== textIsSpace) flushText(); - if (textStart === -1) textIsSpace = isSpace; - startText(); - pos++; - break; - } + case Mode.QuotedValue: { + if (scanLiquidOpen()) continue; - case Mode.QuotedValue: { - if (scanLiquidOpen()) continue; + // Curly quotes are directional, so a value opened with a left curly + // quote closes on its right partner (and vice-versa); straight quotes + // close on themselves. + if (ch(0) === closingQuoteFor(quoteChar) || ch(0) === quoteChar) { + emit(TokenType.HtmlQuoteClose, 1); + popMode(); + continue; + } - // Curly quotes are directional, so a value opened with a left curly - // quote closes on its right partner (and vice-versa); straight quotes - // close on themselves. - if (ch(0) === closingQuoteFor(quoteChar) || ch(0) === quoteChar) { - emit(TokenType.HtmlQuoteClose, 1); - popMode(); - continue; + startText(); + pos++; + break; } - startText(); - pos++; - break; + default: + assertNever(mode); } - - default: - assertNever(mode); } + if (pos < source.length) return true; + if (!ended) { + flushText(); + tokens.push({ type: TokenType.EndOfInput, start: source.length, end: source.length }); + ended = true; + } + return false; } - flushText(); - tokens.push({ type: TokenType.EndOfInput, start: source.length, end: source.length }); - return tokens; + return { restart, fill }; } enum Mode {