Skip to content

fix: make querySelectorAll linear in sibling count and safe on deep nesting - #333

Merged
taoqf merged 1 commit into
taoqf:mainfrom
s-kris:fix/linear-findall
Nov 6, 2026
Merged

taoqf merged 1 commit into
taoqf:mainfrom
s-kris:fix/linear-findall

Conversation

@s-kris

@s-kris s-kris commented Nov 6, 2026 •

Copy link
Copy Markdown

Fixes #332.

What was wrong

findAll in src/matcher.ts, which css-select calls for querySelectorAll, built its result like this:

if (childs) result = result.concat(findAll(test, childs));
  1. Every concat copies all the matches found so far, once per child, so the search was quadratic in the number of siblings.
  2. It recursed once per nesting level, so very deep documents overflowed the call stack.

The change

findAll now walks the tree depth-first with an explicit stack and pushes matches onto one array. Children are pushed in reverse, so they come off the stack in document order. The matches and their order are unchanged.

Measured

querySelectorAll('img') on <div> + N sibling <img>, Node 24:

Siblings Before After
40,000 2,418 ms 4 ms
90,000 12,057 ms 6 ms
1 MB article, 15,000 nested images 95 ms 7 ms

On 10,000 nested <div>s, querySelectorAll('div') threw Maximum call stack size exceeded before the change and returns 10,000 elements after it.

Tests

test/tests/queryselectorall-scaling.js covers three things:

  1. Document order for nested matches, including *.
  2. 100,000 siblings in well under a second. The bound is generous on purpose, so it cannot flake on a slow CI machine, while the old code would take over 15 seconds.
  3. 10,000 levels of nesting without a stack overflow.

bun run test passes: 325 passing, 13 pending, as before. The benchmark's node-html-parser line is unchanged at about 1.06 ms per file.

bun run lint fails on a clean checkout of main too, with Failed to load plugin '@typescript-eslint', so I could not lint this change locally.

…esting

findAll concatenated each child's matches onto the result, copying every
match found so far once per child, so querySelectorAll was quadratic in the
number of siblings (40,000 siblings took about 2.4 seconds). It also recursed
once per nesting level, so 10,000 nested elements overflowed the stack.

Walk the tree depth-first with an explicit stack and a single result array.
Matches are the same and in the same document order.

Closes taoqf#332
@s-kris
s-kris force-pushed the fix/linear-findall branch from 9556514 to e0e109a Compare November 6, 2026 01:15
@taoqf
taoqf merged commit 89eb8bd into taoqf:main Nov 6, 2026
3 checks passed
laithalsaadoon added a commit to memhtml/memhtml that referenced this pull request Nov 11, 2026
Moves `node-html-parser` from 9.0.6 to 9.0.7, the exact pin in `apps/cli/package.json` (manifest and lockfile only; no other lockfile key moves). `memhtml exec` writes this package into its guest as `/workspace/lib/nhp.mjs`, so scripts that agents run over a store parse records with it. memhtml's own writer uses parse5, so nothing memhtml writes changes.

What 9.0.7 changes (taoqf/node-html-parser#333, fixing #332): `findAll`, the css-select adapter behind `querySelectorAll`, used to recurse once per nesting level and rebuild its result with `concat` for every child. It now walks the tree depth-first with an explicit stack and one result array, pushing children in reverse so matches keep document order. The parser, `querySelector`, `.text`, `.structuredText` and `toString` are untouched.

Evidence from a dependency read against main 94c7f3e:

- Published artifact: `dist/index.mjs` differs from 9.0.6 by one hunk inside `findAll`, which is the source change; the `.d.ts` files and dependencies are identical, and the tarball has no new imports, globals or `eval`. Its shasum equals upstream CI's pack of the v9.0.7 tag. Like every earlier release it was published by the sole maintainer without provenance; `npm audit signatures` verifies 11/11.
- Differential over 1,000 generated inputs, 19 handcrafted records and tag-soup cases, and a 1,131-file fixture corpus: identical trees, text and row matrices, and the same nodes in the same order for 41 selectors. A planted mutant (children pushed in forward order) changes 738 of the 1,000 outputs, so the harness sees order changes.
- Through `memhtml exec` on a fixture store, every record gave the same result on both versions except one hand-committed record nested 4,500 levels deep: 9.0.6 threw `InternalError: stack overflow` in the QuickJS guest, and 9.0.7 returns. `querySelector`, `.text` and `.structuredText` still recurse, so their depth limits are unchanged.
- `querySelectorAll` over many matching siblings goes from quadratic to linear: 24,000 `<li>` in the guest took 17.7 to 20.8 s before and 43 to 52 ms now.
- Advisories: none for either version.

Local gate: `mise run check` 76/76 in 264 s on main cdebb40.

Left open for review: an `exec` script's result changes where 9.0.6 threw, which makes this a reached behavior change, so it is typed `fix(deps):` for the release notes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

querySelectorAll is quadratic in sibling count and overflows the stack on deep nesting

2 participants