Skip to content

fix(clone): isolate cloned arrays from source documents - #16281

Merged
vkarpov15 merged 2 commits into
Automattic:masterfrom
AbdelrahmanHafez:fix/clone-parent-index-refresh
May 14, 2026
Merged

fix(clone): isolate cloned arrays from source documents#16281
vkarpov15 merged 2 commits into
Automattic:masterfrom
AbdelrahmanHafez:fix/clone-parent-index-refresh

Conversation

@AbdelrahmanHafez

Copy link
Copy Markdown
Collaborator

re #15958

$clone() was still leaving one parent pointer connected to the source document. We fixed the parent pointers on cloned subdocs, but retained Mongoose arrays were still constructed with the source document as their internal array parent. That meant cloning could mark the original document modified while populating the clone, and later writes to a cloned array could bubble markModified() back to the original.

This PR:

  • uses the cloned parent doc when constructing retained Mongoose arrays
  • populates cloned document arrays without marking either document modified
  • restamps cloned array subdoc indexes while doing that, so stale indexes from prior array edits don't leak into the clone
  • adds regression coverage for document arrays and primitive arrays

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a $clone() isolation bug where cloned Mongoose arrays could still be internally parented to the source document, allowing modifications on the clone (and even clone-time population) to mark the original document as modified. The changes ensure cloned document arrays have correct subdocument indexes after source mutations and that both document arrays and primitive arrays don’t leak modification tracking back to the original.

Changes:

  • Restamp cloned document-array subdocument indexes during clone array population (while suppressing change tracking).
  • Add regression tests to ensure $clone() doesn’t mutate the original document’s modifiedPaths() for primitive arrays and doesn’t propagate modifications from clone to original.
  • Add test coverage for cloning document arrays with null entries and for index correctness after source array edits.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/document.test.js Adds regression coverage for clone isolation: subdoc index correctness, null entries in doc arrays, and primitive array modified state isolation.
lib/helpers/clone.js Restamps subdocument indexes during retained document-array cloning while avoiding marking either source or clone modified.

@AbdelrahmanHafez
AbdelrahmanHafez marked this pull request as ready for review May 13, 2026 19:22
@AbdelrahmanHafez
AbdelrahmanHafez marked this pull request as draft May 13, 2026 19:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@AbdelrahmanHafez
AbdelrahmanHafez marked this pull request as ready for review May 13, 2026 19:52

@hasezoey hasezoey left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vkarpov15 vkarpov15 added this to the 9.6.3 milestone May 14, 2026

@vkarpov15 vkarpov15 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks 👍

@vkarpov15
vkarpov15 merged commit 91de1ae into Automattic:master May 14, 2026
30 checks passed
vkarpov15 pushed a commit that referenced this pull request May 14, 2026
* test(document): cover clone array isolation edge cases

* fix(clone): refresh cloned document array indexes
@vkarpov15

Copy link
Copy Markdown
Collaborator

I also cherry-picked this into 8.x branch.

@vkarpov15 vkarpov15 modified the milestones: 9.6.3, 8.24 May 14, 2026
@AbdelrahmanHafez
AbdelrahmanHafez deleted the fix/clone-parent-index-refresh branch May 14, 2026 17:58
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.

4 participants