Cannot patch a sub-document with destructuring

[a88zach]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Mongoose version

8.1.0

Node.js version

20.11.0

MongoDB server version

7.x

Typescript version (if applicable)

5.3.3

Description

When patching a sub-document with destructuring, the changes are not saved correctly.

const person = await personModel.findOne({...});

person.address = {
      ...person.address,
      zip: 54321,
    };

    await person.save();

However, after saving (and even before the save is called) the person.address.zip is still the old value.

Steps to Reproduce

Cone this repo: https://github.com/a88zach/mongoose-bug

run $ yarn test

Notice the 1 test that is successful and the 1 test that fails. The one that fails is using destructuring to patch the sub document

Expected Behavior

The sub-document should be updated with the existing information plus any overrides based on the patch

[IslandRhythms]

const mongoose = require('mongoose');

const addressSchema = new mongoose.Schema(
  {
    street: String,
    city: String,
    state: String,
    zip: Number,
  },
  { _id: false }
);

const personSchema = new mongoose.Schema({
  name: String,
  age: Number,
  address: addressSchema,
});

const personModel = mongoose.model("Person", personSchema);

async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  await mongoose.connection.dropDatabase();

  const person = new personModel({
    name: "John",
    age: 42,
    address: {
      street: "123 Fake St",
      city: "Springfield",
      state: "IL",
      zip: 12345,
    },
  });

  await person.save();

  
  person.address = {
    ...person.address,
    zip: 54321,
  };

  await person.save();

  console.log('what is person', person);
}

run();

[vkarpov15]

Looks to be a slightly different case of #11522. I would strongly advise you do person.address.zip = 54321; or person.address.set('zip', 54321) instead of using the spread operator. You aren't writing React here, person.address = { ...person.address, zip: 54321 } is just an unnecessary shallow clone that hurts performance. The other workaround is to do the following:

  person.address = {
    ...person.address.toObject(),
    zip: 54321,
  };

We will put in a PR to make the OP's code work, but we recommend against using it.

[a88zach]

@vkarpov15 I would agree with you that I'm not writing React here. However, I am writing javascript here and destructuring has been apart of the spec since es6 was introduced in 2015. There are real-world use cases where destructuring makes more sense than using the alternatives provided above.

Although this is a contrived example. Is you make person.address optional and all address fields optional in the above example, destructuring helps consolidate code like:

if (person.address) {
  person.address.zip = 54321;
} else {
  person.address = {
    zip: 54321;
  }
}

Obviously, this is just an example, but with more complex code bases, this pattern surely applies.

[SinBirb]

I agree with @a88zach, it is not obvious that upgrading from Mongoose 5 to 8 will break a core JavaScript functionality. That fix actually makes it worse because now it kind of works, but still not how everyone, except Mongoose developers, would expect it.

[vkarpov15]

For the following code:

if (person.address) {
  person.address.zip = 54321;
} else {
  person.address = {
    zip: 54321;
  }
}

The more concise way in Mongoose would be person.set('address.zip', 54321). Oddly enough person.address.zip = 54321 also works, because address is a nested path and nested paths are never undefined on Mongoose documents, but relying on the fact that nested paths are never undefined is not very readable IMO because it isn't obvious why that pattern works without deep understanding of Mongoose.

I agree that person.address = { ...person.address, zip: 54321 } should work, and we're working on a more complete fix. I'm just trying to emphasize the point that, ideally, you shouldn't use this spread operator pattern for performance reasons: not only do you have an unnecessary shallow clone of a complex object, but you're also telling Mongoose to do a deep diff, and update the entire address subdocument in MongoDB. Whereas all you want to do is just update the address.zip property.