[6.x regression / behavior change?] findOneAndUpdate no longer sets undefined fields as null

[ronjouch]

I confirm I am using the latest version of mongoose as of today: 6.0.14

Do you want to request a feature or report a bug?

Unsure. Maybe this is a 6.x bug/regression, but maybe this is a desired behavior change. Halp, please: is this change desired, and was it documented? I couldn't find a trace of it, neither in the mongoose / Migrating from 5.x to 6.x nor in the underlying node-mongodb-native / Changes in 4.x (and how to migrate!) docs.

Steps to reproduce

#!/usr/bin/env node
const mongoose = require('mongoose');
const mongooseVersion = require('./node_modules/mongoose/package.json').version;
const mongooseMajorVersion = parseInt(mongooseVersion.substring(0, 1), 10);
const mongooseOptions = mongooseMajorVersion === 6 ? {} : { useNewUrlParser: true, useUnifiedTopology: true, useFindAndModify: false };
mongoose.connect('mongodb://localhost:27017/test', mongooseOptions);

const Cat = mongoose.model('Cat', { name: String, age: Number });

async function testCase() {
  console.log('Using mongoose @', mongooseVersion);

  // Cleanup for re-runnability
  await Cat.deleteMany({ name: 'Zildjian' });
  const freshZildjian = await Cat.create({ name: 'Zildjian' })
  console.log('Zildjian was born:', JSON.stringify(freshZildjian, null, 2));

  // What matters
  const updatedZildjian = await Cat.findOneAndUpdate(
    { name: 'Zildjian' }, // Will *NOT* be found, so this findOneAndUpdate will always upsert
    { $set: { age: undefined } }, // What matters: we attempt to set a key with an undefined value
    { new: true },
  )
  console.log('Updated Zildjian:', JSON.stringify(updatedZildjian, null, 2));
  // Difference between Mongoose 5 and 6:
  // - Mongoose 5 used to return a cat with `"age": null`
  // - Mongoose 6 returns a cat with no `age`
  // Desired change, or regression?
}

testCase().then(() => {
  mongoose.disconnect();
  process.exit(0);
})

What is the current behavior?

Mongoose 6 findOneAndUpdate does not set undefined values

Using mongoose @ 6.0.14
Zildjian was born: {
  "name": "Zildjian",
  "_id": "61a8f24bcdb4dcd14f0ecd03",
  "__v": 0
}
Updated Zildjian: {
  "_id": "61a8f24bcdb4dcd14f0ecd03",
  "name": "Zildjian",
  "__v": 0
  // Mongoose 6 does *not* set an `age` key/value (which was passed with value `undefined`)
}

What is the expected behavior?

Mongoose 5 findOneAndUpdate used to set undefined values as null:

Using mongoose @ 5.13.13
Zildjian was born: {
  "_id": "61a8f23dd1891e9f846319d4",
  "name": "Zildjian",
  "__v": 0
}
Updated Zildjian: {
  "_id": "61a8f23dd1891e9f846319d4",
  "name": "Zildjian",
  "__v": 0,
  "age": null  // <-- HERE: Mongoose 5 sets the `undefined` age value as `null`
}

→ Is this change desired, and was it documented? I couldn't find a trace of it, neither in the mongoose / Migrating from 5.x to 6.x nor in the underlying node-mongodb-native / Changes in 4.x (and how to migrate!) docs.

What are the versions of Node.js, Mongoose and MongoDB you are using?

• Node.js 14.18.2
• Mongoose 5.13.13 / 6.0.14
• MongoDB 5.0.4

[IslandRhythms]

const mongoose = require('mongoose');

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

const Test = mongoose.model('Test', testSchema);

async function run() {
    await mongoose.connect('mongodb://localhost:27017/', {useNewUrlParser: true,
    useUnifiedTopology: true,});
    await mongoose.connection.dropDatabase();

    await Test.create({ name: 'Test'});

    let entry = await Test.findOneAndUpdate({
        name: 'Test',
    }, {age: undefined}, {new: true});

    console.log(entry);

    console.log(await Test.find());


};

run();

[vkarpov15]

This is expected behavior in 6.x, we removed the omitUndefined option and made it true always. That means Mongoose will always strip undefined keys from updates. There's an entry in the changelog, and we'll add a note to the migration guide with #10672.

[ronjouch]

This is expected behavior in 6.x, we removed the omitUndefined option and made it true always. That means Mongoose will always strip undefined keys from updates. There's an entry in the changelog

Indeed: BREAKING CHANGE: remove omitUndefined option for updates - Mongoose now always removes undefined keys in updates #7680

we'll add a note to the migration guide with #10672

@vkarpov15 yes please, migration guide entry very welcome. The existence of a migration guide implies to me (and I don't think that's unreasonable) that it's the thing to read rather than skimming through the detailed changelog, as it may merge/explicit breaking changes to be more easily digestible by users. So, 💯 it should cover this breaking change.

Thanks for listening.

[vkarpov15]

We added it to the migration guide, sorry for the inconvenience. We try our best to make sure the migration guide is exhaustive, but in this case we missed a spot.

[joebowbeer]

@vkarpov15 For the migration guide and my benefit, please explain how to recreate the previous behavior using the new version of mongoose. Are there global or local options that can be set? $setOnInsert?

We have test code that depends on the age query above returning null and I would like to avoid having to upgrade the whole world at once.

So far I have been able to avoid upgrading the world by using global options:

• setDefaultsOnInsert: false
• autoCreate: false

But in this case omitUndefined was completely removed so I am at a loss.

[vkarpov15]

@joebowbeer can you please show me an example of what the test code in question looks like? I'd like to see if I can suggest a workaround without reintroducing omitUndefined.

[joebowbeer]

@vkarpov15 The test code is an e2e test. It sends an event that causes the server-under-test to set an entry with an undefined value and then it verifies that the value is null.

The server-under-test code is similar to the code in the repro steps provided in the description and in the comment above.

The test code is essentially just verifying that the results are as expected: that the cat has age: null.

In our case the server-under-test was updated to mongoose6 but the test wasn't, and consequently the test fails.

My question is: Is there any way to configure the updated server-under-test to reproduce the old behavior? If not, then anything intentionally or unintentionally dependent on its previous behavior may also need to be updated in sync.

Should I create a new issue to enhance the documentation?

[vkarpov15]

There's no way to configure server-under-test to repro the old behavior currently. If you're asserting that updating a key to undefined makes that key strictly equal to null, no real workaround other than to set the key to null yourself.

What suggestions do you have for improving the documentation around this?

[ronjouch]

OP here. I'll chime in that I'm in a similar boat of current behavior "broken" by the change. That being said, it was easy enough to patch our code, from setting nulls through implicit behavior to setting nulls explicitly.

I tend to provide/expect a high level of backwards compat and sometimes rant about it, but in this case it's very marginal behavior and AFAIK wasn't even documented initially. Said differently, since there was no contract about thing, and since we depended (knowingly or not) on thing anyway, it's our users task to adjust to new thing, and I don't expect anything more in documentation than "behavior changed, please set your nulls explicitly if you depend on the previous behavior".

[joebowbeer]

@vkarpov15 thanks for clarifying. If there was a way to achieve strict compatibility, then that would be worth documenting.

I think the suggested text is a good addition:

please set your nulls explicitly if you depend on the previous behavior

[vkarpov15]

@ronjouch I agree wholeheartedly about backwards compat. But this was a case where we think that backwards compat means supporting surprising and confusing behavior - changing undefined to null has been a common cause of confusion.

We'll add some more info to the docs about this. Not sure exactly where, we'll have to take a closer look.

[ronjouch]

Fantastic. Thanks Valeri.