fix: correct handling of relative vs absolute paths with maps and subdocuments#15682
fix: correct handling of relative vs absolute paths with maps and subdocuments#15682
Conversation
…ss a relative path to the subdocument markModified
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical issue with change tracking in nested map and subdocument structures where paths were inconsistently handled between absolute and relative contexts. The fix ensures that both maps and subdocuments properly track both their full path (relative to root document) and their path relative to their parent, using the appropriate path type for change tracking operations.
Key changes:
- Enhanced map constructor to calculate and store both full paths and relative-to-parent paths
- Modified subdocument path handling to use stored relative paths when available
- Updated change tracking logic to use the correct path type based on parent context
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/types.map.test.js | Adds comprehensive test case for nested maps with document arrays to verify correct change tracking |
| lib/types/subdocument.js | Updates subdocument path handling and markModified logic to use relative paths efficiently |
| lib/types/map.js | Enhances map constructor and set method to properly handle both absolute and relative paths |
| lib/types/array/methods/index.js | Fixes array method path handling for subdocument parents |
| lib/schema/subdocument.js | Passes path options through subdocument constructor |
| lib/schema/map.js | Updates map casting to use calculated full paths and pass options correctly |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Thank you for your quick work on fixing this issue! I tested against your PR branch and it does fix the incorrect document paths. However now that doc updates are able to be properly generated, I'm running into a few more issues with change tracking/delta calculations which appear to be related to array types:
See the following repro (against PR branch): const itemSchema = new Schema({
numField: Number,
arrayField: [String],
}, { _id: false });
const groupSchema = new Schema({
items: { type: Map, of: itemSchema },
numField: Number,
arrayField: [String],
}, { _id: false });
const parentSchema = new Schema({
groups: { type: Map, of: groupSchema },
singleItem: { type: itemSchema },
});
const M = model("parentSchema", parentSchema);
const x = new M().init({
groups: {
g1: {
items: {
i1: {
numField: 1,
arrayField: ["a"],
},
},
numField: 2,
arrayField: ["b"],
},
},
singleItem: {
numField: 3,
arrayField: ["c"],
},
});
x.singleItem.arrayField.push("val");
// entire subdocument is set
// {"$set":{"singleItem":{"numField":3,"arrayField":["c","val"]}}}
x.groups.get("g1").arrayField.push("val");
// entire subdocument is set and existing map entries are lost
// {"$set":{"groups.g1":{"items":{},"numField":2,"arrayField":["b","val"]}}}
x.groups.get("g1").items.get("i1").arrayField.push("val");
// {"$set":{"groups.g1.items.i1":{"numField":1,"arrayField":["a","val"]}}}
x.singleItem.arrayField.pull("c");
// when in a regular subdocument, doesn't use the correct operator but otherwise works
// {"$set":{"singleItem":{"numField":3,"arrayField":[]}}}
x.groups.get("g1").arrayField.pull("b");
// but in a map entry..
// ./node_modules/mongoose/lib/types/array/methods/index.js:623
// let i = cur.length;
// ^
// TypeError: Cannot read properties of undefined (reading 'length')
// at Proxy.pull (./node_modules/mongoose/lib/types/array/methods/index.js:623:17)If you prefer I can open a new issue for this. |
|
@hasezoey @kballen1 please take another look, I fixed this to account for the new cases @kballen1 mentioned and a couple of others. I double checked and performance on the |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
All cases look good to me. Thank you! |
 <h3>Snyk has created this PR to upgrade mongoose from 8.19.1 to 8.19.2.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **1 version** ahead of your current version. - The recommended version was released **a month ago**. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>mongoose</b></summary> <ul> <li> <b>8.19.2</b> - <a href="https://redirect.github.com/Automattic/mongoose/releases/tag/8.19.2">2025-10-20</a></br><h1>8.19.2 / 2025-10-20</h1> <ul> <li>perf(setDefaultsOnInsert): avoid computing all modified paths when running setDefaultsOnInsert and update validators, only calculate if there are defaults to set <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3529183304" data-permission-text="Title is private" data-url="Automattic/mongoose#15691" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15691/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15691">#15691</a> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3481017639" data-permission-text="Title is private" data-url="Automattic/mongoose#15672" data-hovercard-type="issue" data-hovercard-url="/Automattic/mongoose/issues/15672/hovercard" href="https://redirect.github.com/Automattic/mongoose/issues/15672">#15672</a></li> <li>fix: correct handling of relative vs absolute paths with maps and subdocuments <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3500923735" data-permission-text="Title is private" data-url="Automattic/mongoose#15682" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15682/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15682">#15682</a> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3484324954" data-permission-text="Title is private" data-url="Automattic/mongoose#15678" data-hovercard-type="issue" data-hovercard-url="/Automattic/mongoose/issues/15678/hovercard" href="https://redirect.github.com/Automattic/mongoose/issues/15678">#15678</a> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2990690538" data-permission-text="Title is private" data-url="Automattic/mongoose#15350" data-hovercard-type="issue" data-hovercard-url="/Automattic/mongoose/issues/15350/hovercard" href="https://redirect.github.com/Automattic/mongoose/issues/15350">#15350</a></li> <li>ci: add publish script with provenance <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3504550328" data-permission-text="Title is private" data-url="Automattic/mongoose#15684" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15684/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15684">#15684</a> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3491146472" data-permission-text="Title is private" data-url="Automattic/mongoose#15680" data-hovercard-type="issue" data-hovercard-url="/Automattic/mongoose/issues/15680/hovercard" href="https://redirect.github.com/Automattic/mongoose/issues/15680">#15680</a></li> </ul> </li> <li> <b>8.19.1</b> - <a href="https://redirect.github.com/Automattic/mongoose/releases/tag/8.19.1">2025-10-06</a></br><h1>8.19.1 / 2025-10-06</h1> <ul> <li>perf: avoid getting all modified paths in update when checking if versionKey needs to be set <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3484140102" data-permission-text="Title is private" data-url="Automattic/mongoose#15677" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15677/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15677">#15677</a> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3481017639" data-permission-text="Title is private" data-url="Automattic/mongoose#15672" data-hovercard-type="issue" data-hovercard-url="/Automattic/mongoose/issues/15672/hovercard" href="https://redirect.github.com/Automattic/mongoose/issues/15672">#15672</a></li> <li>perf: Avoid needless path translation <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3484417969" data-permission-text="Title is private" data-url="Automattic/mongoose#15679" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15679/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15679">#15679</a> <a href="https://redirect.github.com/orgads">orgads</a></li> <li>fix(query): throw error if using update operator with modifier and no path <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3478432282" data-permission-text="Title is private" data-url="Automattic/mongoose#15670" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15670/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15670">#15670</a> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3444264689" data-permission-text="Title is private" data-url="Automattic/mongoose#15642" data-hovercard-type="issue" data-hovercard-url="/Automattic/mongoose/issues/15642/hovercard" href="https://redirect.github.com/Automattic/mongoose/issues/15642">#15642</a></li> <li>types: avoid making FilterQuery a conditional type because of how typescript handles distributed conditional unions <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3484101141" data-permission-text="Title is private" data-url="Automattic/mongoose#15676" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15676/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15676">#15676</a> <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3479049514" data-permission-text="Title is private" data-url="Automattic/mongoose#15671" data-hovercard-type="issue" data-hovercard-url="/Automattic/mongoose/issues/15671/hovercard" href="https://redirect.github.com/Automattic/mongoose/issues/15671">#15671</a></li> <li>docs: update installation instructions <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3483906022" data-permission-text="Title is private" data-url="Automattic/mongoose#15675" data-hovercard-type="pull_request" data-hovercard-url="/Automattic/mongoose/pull/15675/hovercard" href="https://redirect.github.com/Automattic/mongoose/pull/15675">#15675</a> <a href="https://redirect.github.com/aalok-y">aalok-y</a></li> </ul> </li> </ul> from <a href="https://redirect.github.com/Automattic/mongoose/releases">mongoose GitHub release notes</a> </details> </details> --- > [!IMPORTANT] > > - Check the changes in this PR to ensure they won't cause issues with your project. > - This PR was automatically created by Snyk using the credentials of a real user. --- **Note:** _You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs._ **For more information:** <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiI2NTRiNWJjYy05YWVjLTQ5MDEtYmYxOS0xZDhlNWFlOTQ2NmQiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjY1NGI1YmNjLTlhZWMtNDkwMS1iZjE5LTFkOGU1YWU5NDY2ZCJ9fQ==" width="0" height="0"/> > - 🧐 [View latest project report](https://app.snyk.io/org/nadae1010/project/4b43d5cd-fce2-45c9-8f9b-f145417ebc0e?utm_source=github&utm_medium=referral&page=upgrade-pr) > - 📜 [Customise PR templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates?utm_source=&utm_content=fix-pr-template) > - 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/nadae1010/project/4b43d5cd-fce2-45c9-8f9b-f145417ebc0e/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) > - 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/nadae1010/project/4b43d5cd-fce2-45c9-8f9b-f145417ebc0e/settings/integration?pkg=mongoose&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) [//]: # 'snyk:metadata:{"breakingChangeRiskLevel":null,"FF_showPullRequestBreakingChanges":false,"FF_showPullRequestBreakingChangesWebSearch":false,"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"mongoose","from":"8.19.1","to":"8.19.2"}],"env":"prod","hasFixes":false,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":[],"prId":"654b5bcc-9aec-4901-bf19-1d8e5ae9466d","prPublicId":"654b5bcc-9aec-4901-bf19-1d8e5ae9466d","packageManager":"npm","priorityScoreList":[],"projectPublicId":"4b43d5cd-fce2-45c9-8f9b-f145417ebc0e","projectUrl":"https://app.snyk.io/org/nadae1010/project/4b43d5cd-fce2-45c9-8f9b-f145417ebc0e?utm_source=github&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":[],"type":"auto","upgrade":[],"upgradeInfo":{"versionsDiff":1,"publishedDate":"2025-10-20T18:49:01.560Z"},"vulns":[]}'
Fix #15678
Re: #15350
Summary
The core issue in #15678 and #15350 is that maps use absolute paths for change tracking (path relative to the top-level document), whereas subdocuments use relative paths (path relative to parent subdocument, change tracking bubbles up). This inconsistency becomes problematic when you have multiple layers of nesting - in #15678, the issue was a subdocument underneath a map underneath another map.
In this PR, maps and subdocuments now both track pathRelativeToParent as well as path (full/absolute path) and use the correct path for change tracking.
Examples