Skip to content

Commit 98be4f1

Browse files
perf(components): scan for component tags without copying the document per tag (#1945)
`findComponentTags` walks the document looking at every `<`. To test whether a tag name matched, it sliced the whole remaining document and ran an anchored `^...` pattern against the copy: const afterLt = html.slice(tagStart + 1) const m = afterLt.match(new RegExp(`^(${tagPattern.source})(?![\w.:-])`)) Both halves are per-`<`: a copy of the rest of the page, and a regex compile. It runs three times per pass (kebab, PascalCase, lowercase) and again for every nested component. A sticky matcher does the same test at an offset in the original string. Same pattern, same lookahead, no copy, compiled once per source rather than once per character. Measured with a String.prototype allocation profiler over one steady-state render, on four fixture shapes: before after component-dense 107.57MB 14.28MB -87% expression-dense 18.88MB 3.83MB -80% stores + composables 6.02MB 3.91MB -35% plain signals page 3.53MB 3.53MB 0% (no component tags) On the component-dense page this one site was 95.5MB of the 107.6MB the render allocated -- 89% of it, 4,250 slices averaging 23KB each. It is the largest single item behind this issue by a wide margin; the three commits before this one moved 377KB between them. The plain-signals row is not a disappointment, it is the control: a page with no component tags never entered the loop, and its number does not move. Three things the rewrite had to preserve, each pinned by a sabotage run: The #1845 lookahead. `(?![\w.:-])` is what stops the pattern matching a PREFIX of a longer tag -- `<ion-button />` matched `ion`, lost the hyphen that marks it a custom element, and resolved `ion.stx` from disk, splicing an ENOENT into the page. Dropping it from the shared matcher fails 3 component tests. The offset. A sticky regex matches at `lastIndex`, so it must be set to `tagStart + 1` before every exec. Leaving it at 0 fails 78 tests. Flags. The old matcher was built from `.source` alone, so it was case-sensitive whatever the caller's pattern said, and the kebab/Pascal/lowercase dispatch depends on that. The sticky matcher adds `y` and nothing else. The new matchers are cached by pattern source. The three call sites declare their patterns as literals inside a function, so each call passes a fresh RegExp object with a familiar source -- keying on the object would miss every time, and keying on the source holds three entries. A caller building patterns dynamically would need to bound it; none does. Verified byte-identical against HEAD across 12 renders (4 fixture shapes x SEO on / SEO off / colorMode). 5 new tests covering the sticky-index risks the existing suite does not reach. Suite 12,709 pass / 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 132ef77 commit 98be4f1

2 files changed

Lines changed: 111 additions & 9 deletions

File tree

packages/stx/src/component-processing.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,44 @@ export interface ComponentTagMatch {
9191
isSelfClosing: boolean
9292
}
9393

94+
/**
95+
* Sticky tag-name matchers, one per pattern source.
96+
*
97+
* `findComponentTags` tests every `<` in the document, and it used to do that
98+
* by slicing the whole remaining document and matching `^...` against the copy.
99+
* On a component-dense 237KB page that was 4,250 slices totalling 95MB in a
100+
* single render -- 89% of everything the render allocated, and the largest
101+
* single item behind stacksjs/stx#1945. A sticky match runs at an offset in the
102+
* original string and copies nothing.
103+
*
104+
* Compiling once per pattern instead of once per `<` takes a regex compile out
105+
* of the same loop. The three call sites pass module-level literals, so this
106+
* map holds three entries; a caller that builds patterns dynamically would need
107+
* to bound it.
108+
*
109+
* Flags are deliberately NOT carried over from `tagPattern`. The old matcher
110+
* was built from `.source` alone, so it was always case-sensitive whatever the
111+
* caller's pattern said -- and the PascalCase / kebab / lowercase dispatch
112+
* depends on that.
113+
*
114+
* The lookahead is the #1845 fix and has to stay: a tag name ends at the first
115+
* character that cannot appear in one. Without it the pattern matches a PREFIX
116+
* of a longer tag -- `<ion-button />` matched `ion`, lost the hyphen that marks
117+
* it a custom element, and resolved `ion.stx` from disk, splicing an ENOENT
118+
* error into the page. The paired form `<ion-button></ion-button>` was already
119+
* ignored, so only self-closing custom elements were affected.
120+
*/
121+
const tagNameMatchers = new Map<string, RegExp>()
122+
123+
function tagNameMatcherFor(tagPattern: RegExp): RegExp {
124+
let matcher = tagNameMatchers.get(tagPattern.source)
125+
if (!matcher) {
126+
matcher = new RegExp(`(${tagPattern.source})(?![\\w.:-])`, 'y')
127+
tagNameMatchers.set(tagPattern.source, matcher)
128+
}
129+
return matcher
130+
}
131+
94132
/**
95133
* Find component tags in HTML, properly handling quoted strings
96134
* This solves the issue where `>` inside attribute values would incorrectly end the tag
@@ -110,15 +148,10 @@ export function findComponentTags(html: string, tagPattern: RegExp, skipTags?: S
110148
if (tagStart === -1)
111149
break
112150

113-
// Check if this matches our tag pattern
114-
const afterLt = html.slice(tagStart + 1)
115-
// A tag name ends at the first character that cannot appear in one.
116-
// Without this lookahead the pattern matches a PREFIX of a longer tag:
117-
// `<ion-button />` matched `ion`, lost the hyphen that marks it a custom
118-
// element, and resolved `ion.stx` from disk — splicing an ENOENT error
119-
// into the page. The paired form `<ion-button></ion-button>` was already
120-
// ignored, so only self-closing custom elements were affected (#1845).
121-
const tagNameMatch = afterLt.match(new RegExp(`^(${tagPattern.source})(?![\\w.:-])`))
151+
// Check if this matches our tag pattern, at that offset in `html` itself.
152+
const matcher = tagNameMatcherFor(tagPattern)
153+
matcher.lastIndex = tagStart + 1
154+
const tagNameMatch = matcher.exec(html)
122155

123156
if (!tagNameMatch) {
124157
pos = tagStart + 1
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* `findComponentTags` scans without copying (stacksjs/stx#1945).
3+
*
4+
* The scan tests every `<` in the document. It used to do that by slicing the
5+
* whole remaining document and matching an anchored pattern against the copy:
6+
* on a component-dense 237KB page, 4,250 slices totalling 95MB in one render --
7+
* 89% of everything that render allocated, and the single largest item behind
8+
* the issue. It matches at an offset in the original string now.
9+
*
10+
* The behaviour that has to survive is pinned by the component suite at large;
11+
* what is pinned HERE is the risk the rewrite introduces and nothing else
12+
* covers: a sticky regex carries `lastIndex` between calls, and the matcher is
13+
* now shared across calls rather than rebuilt per `<`. A stale index silently
14+
* skips tags near the start of the next document.
15+
*/
16+
17+
import { describe, expect, it } from 'bun:test'
18+
import { findComponentTags } from '../../src/component-processing'
19+
20+
const PASCAL = /[A-Z][a-zA-Z0-9]*/
21+
22+
describe('component tag scanning', () => {
23+
it('finds a tag at the very start of the document', () => {
24+
// Offset 0 is where a leaked lastIndex bites first.
25+
const tags = findComponentTags('<Card />', PASCAL)
26+
27+
expect(tags).toHaveLength(1)
28+
expect(tags[0].tagName).toBe('Card')
29+
expect(tags[0].startIndex).toBe(0)
30+
})
31+
32+
it('gives the same answer on a second call with a fresh pattern object', () => {
33+
// The three callers declare their patterns inside the function, so every
34+
// call passes a NEW RegExp with the same source -- the matcher is looked up
35+
// by source, which is what makes reuse (and lastIndex) possible at all.
36+
const html = '<Card /><Badge />'
37+
const first = findComponentTags(html, /[A-Z][a-zA-Z0-9]*/)
38+
const second = findComponentTags(html, /[A-Z][a-zA-Z0-9]*/)
39+
40+
expect(second.map(t => [t.tagName, t.startIndex])).toEqual(first.map(t => [t.tagName, t.startIndex]))
41+
expect(second.map(t => t.tagName)).toEqual(['Card', 'Badge'])
42+
})
43+
44+
it('does not carry an index over from a longer previous document', () => {
45+
findComponentTags(`${'<p>filler</p>'.repeat(40)}<Card />`, PASCAL)
46+
const tags = findComponentTags('<Badge />', PASCAL)
47+
48+
expect(tags.map(t => t.tagName)).toEqual(['Badge'])
49+
})
50+
51+
it('still refuses a prefix of a longer tag name', () => {
52+
// #1845: `<ion-button />` matched `ion`, lost the hyphen that marks it a
53+
// custom element, and resolved ion.stx from disk -- splicing an ENOENT
54+
// error into the page. The lookahead that prevents it moved into the
55+
// shared matcher, so it is worth asserting where it now lives.
56+
const tags = findComponentTags('<ion-button />', /[a-z][a-z0-9]*/)
57+
58+
expect(tags).toHaveLength(0)
59+
})
60+
61+
it('reports offsets into the original string, not a slice of it', () => {
62+
const prefix = '<div class="wrap">\n '
63+
const tags = findComponentTags(`${prefix}<Card title="x" />\n</div>`, PASCAL)
64+
65+
expect(tags).toHaveLength(1)
66+
expect(tags[0].startIndex).toBe(prefix.length)
67+
expect(tags[0].fullMatch).toBe('<Card title="x" />')
68+
})
69+
})

0 commit comments

Comments
 (0)