Skip to content

Walk sibling and ancestor axes through one helper - #129

Merged
jakejackson1 merged 2 commits into
mainfrom
fix/issue-115
Oct 26, 2026
Merged

jakejackson1 merged 2 commits into
mainfrom
fix/issue-115

Conversation

@gravitypdf-bot

@gravitypdf-bot gravitypdf-bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

The sibling and ancestor traversals in QueryFilters were each a hand-written copy of one loop. They differed only in the DOM property they followed and in when they stopped, so a fix to one had to be repeated in its mirror (#72 had to touch every copy).

What changed

One private helper now walks an axis from each matched element and collects the elements it passes:

private function walk(string $axis, $selector, bool $first, bool $inclusive = false): SplObjectStorage
// $axis: 'nextSibling' | 'previousSibling' | 'parentNode'
// $first: collect the first element matching $selector; otherwise collect every element before the first match
Method Call
next() / prev() walk('nextSibling' / 'previousSibling', $selector, true)
nextAll() / prevAll() filterCandidates(walk(…Sibling, null, false), $selector)
nextUntil() / prevUntil() walk(…Sibling, $selector, false)
parent() walk('parentNode', $selector, true)
parents() filterCandidates(sortReverseDocumentOrder(walk('parentNode', null, false)), $selector)
parentsUntil() sortReverseDocumentOrder(walk('parentNode', $selector, false))
closest() walk('parentNode', $selector, true, true)

*All() is *Until() with nothing to stop at, filtered afterwards. Every walk that is not first-match stops at an element an earlier starting element already collected, because everything beyond it was collected with it, up to the same stop.

getParentElements() is gone. Ordering and selector filtering stay in the public methods; walk() only walks. siblings() keeps its own loop, because it iterates the parent's children instead of walking an axis. Its results still go through the same filterCandidates().

Results are unchanged. I checked this two ways: the existing suite (including Issue62Test) passes untouched, and the output of 45 traversal calls on multi-element sets, with and without selectors, is byte-identical before and after.

Side effects (edge cases, now consistent)

  • parentsUntil() on an element that is not attached to the document used to loop forever, reading nodeType of null. It now stops at the top of the detached subtree, as parents() already did.
  • nextUntil('') and prevUntil('') threw ParseException. They now treat an empty selector as no selector, as parentsUntil(''), next('') and parent('') already did, and as jQuery does.
  • closest('') threw ParseException, and closest(null) threw a TypeError. Both now return an empty set, as jQuery's closest('') does.
  • parents(), nextUntil(), prevUntil() and parentsUntil() now stop walking at the first element already collected from an earlier starting element, as nextAll() and prevAll() already did. nextUntil() on each of 2000 siblings falls from about 97 ms to 0.7 ms (nextUntil('#none'): 212 ms to 6 ms).

Tests

The existing tests are unchanged. Three tests are added to DOMQueryTest:

  • testParentsUntilOutsideTheDocument (fails on main)
  • testTraversalTreatsAnEmptySelectorAsNone (fails on main)
  • testUntilWalksFromElementsOnEachOthersPath, which pins the early stop for *Until() walks that start on each other's paths

vendor/bin/phpunit: 693 tests, 1935 assertions, 0 failures (1 skipped). composer run lint and composer run lint:min-php are clean.

Rebased onto #128

Rebased onto main with #127 and #128. The only conflict was the CHANGELOG; both entries are kept. #128 made NodeMatcher return nodes in input order, which makes filtering before or after the reverse-document sort equivalent; parents() sorts first, as before.

Fixes #115.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.12%. Comparing base (6ae6acf) to head (bff4d32).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #129      +/-   ##
============================================
+ Coverage     90.69%   91.12%   +0.43%     
- Complexity     1424     1431       +7     
============================================
  Files            25       26       +1     
  Lines          3170     3167       -3     
============================================
+ Hits           2875     2886      +11     
+ Misses          295      281      -14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gravitypdf-bot and others added 2 commits October 26, 2026 15:48
next(), prev(), nextAll(), prevAll(), nextUntil(), prevUntil(),
parent(), parents(), parentsUntil() and closest() were each a copy of
the same loop. They now share QueryFilters::walk().

Fixes #115.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
'all' was 'until' with no stop selector, so walk() now takes a bool and
the *Until() methods share nextAll()'s stop at an already-collected
element. Ordering and filtering move back to the callers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jakejackson1 jakejackson1 added the enhancement New feature or request label Oct 26, 2026
@jakejackson1
jakejackson1 merged commit b7429e2 into main Oct 26, 2026
14 checks passed
@jakejackson1
jakejackson1 deleted the fix/issue-115 branch October 26, 2026 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consolidate the sibling and ancestor traversal loops in QueryFilters into one helper

2 participants