Skip to content

Stop inst() deep-copying the match set it replaces - #157

Open
gravitypdf-bot wants to merge 4 commits into
mainfrom
fix/issue-137
Open

gravitypdf-bot wants to merge 4 commits into
mainfrom
fix/issue-137

Conversation

@gravitypdf-bot

Copy link
Copy Markdown
Contributor

Fixes #137.

Every method that returns a new object (find(), children(), filter(), eq(), …) went through inst(), which used clone $this. __clone() deep-copied every node in the match set, and inst() then threw the copies away. inst() now sets a private flag that makes __clone() skip the copy. clone $qp still deep-copies, as documented.

On a 50,000-paragraph document, timings on PHP 8.5:

Call Before After
children() from the root 27 ms 12 ms
find('#nope') 54 ms 39 ms
clone $qp 12.2 ms 12.6 ms (unchanged)

Side effect: end() now returns the original document nodes. Before, it returned the detached copies that __clone() had made.

Tests

tests/Issues/Issue137Test.php checks that:

  • end() returns the original nodes after find(), children(), filter(), eq() and not().
  • Edits made through end() reach the document.
  • clone still deep-copies.

Every PR in this set adds its CHANGELOG line at the end of # Unreleased changes, so each one after the first needs a one-line conflict resolved there.

🤖 Generated with Claude Code

gravitypdf-bot and others added 4 commits October 26, 2026 17:18
inst() cloned the object, and __clone() deep-copied every match with
cloneNode(true), only for inst() to swap the match set straight after. Each
find() from the root copied the whole document, and every chained call
copied whatever was in hand. The copies also became the previous match set,
so end() returned detached copies rather than the nodes the query started
from.

inst() now marks the clone as shallow so __clone() skips the copy; clone $qp
still deep-copies the matches.

Fixes #137

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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.24%. Comparing base (6ae6acf) to head (b125c3c).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #157      +/-   ##
============================================
+ Coverage     90.69%   91.24%   +0.55%     
- Complexity     1424     1442      +18     
============================================
  Files            25       26       +1     
  Lines          3170     3188      +18     
============================================
+ Hits           2875     2909      +34     
+ Misses          295      279      -16     

☔ 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.

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.

Every inst() deep-copies the match set, so each find() from the root copies the whole document

1 participant