Skip to content

Conversation

vkarpov15
Copy link
Collaborator

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

@vkarpov15 vkarpov15 added this to the 8.19.2 milestone Oct 9, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

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

// Calculate the full path from the root document
// Priority: parent.$basePath (from subdoc) > options.path (from parent map/structure) > path (schema path)
// Subdocuments have the most up-to-date path info, so prefer that over options.path
if (this.$__parent?.$isSingleNested && this.$__parent.$basePath) {
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

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

[nitpick] Using optional chaining with logical AND can be simplified. Consider this.$__parent?.$isSingleNested && this.$__parent?.$basePath to be more explicit about null safety for both property accesses.

Suggested change
if (this.$__parent?.$isSingleNested && this.$__parent.$basePath) {
if (this.$__parent?.$isSingleNested && this.$__parent?.$basePath) {

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

LGTM

@kballen1
Copy link

kballen1 commented Oct 12, 2025

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:

  • Arrays in subdocuments (whether standalone or nested in a map) will not emit the proper update operator for push/pulls and instead use $set. Additionally, they seem to mark the entire parent doc as changed, resulting in undesired fields being included in the $set.
  • Array.pull() crashes for paths nested in a map.

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.

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.

Changed paths are incorrect in schemas with nested Maps

3 participants