Adding lean() causes TypeError: document.data is not a function

[jamesholcomb]

Tested with https://github.com/codepunkt/mongoose-patch-history#b9d8abde29d0b1df5137f8c4565a73069aeff3d0

Perhaps this cannot be supported and should be mentioned in the README?

    it('with findOneAndUpdate upsert', (done) => {
      Post.findOneAndUpdate(
        { title: 'findOneAndUpdateUpsert' },
        { $set: { title: 'findOneAndUpdateUpsert' } },
        {
          new: true,
          upsert: true,
          setDefaultsOnInsert: true,
          runValidators: true
        }
      )
        .lean()
        .then((post) =>
          post.patches
            .find({ ref: post._id })
            .sort({ _id: 1 })
            .then((patches) => {
              assert.equal(patches.length, 1)
            })
        )
        .then(done)
        .catch(done)
    })

[vinerich]

Hey @jamesholcomb,
thanks for letting us know!

I think we can support this. I'll have a deeper look over the weekend.

[jamesholcomb]

Additionally, specifying rawResult: true does not work either. It's related to this issue since the virtual functions (e.g. data()) are not provided when the raw result from the driver is returned in the schema.post function.

A simple test already exists for rawResult, but it does not actually assert the patch history.

[vinerich]

Hey @jamesholcomb i know some time has passed by I finally made the time to dig through all open issues. Hopefully you still somewhat know what this was about.

In my understanding lean() and rawResult: true is mostly used for performance or raw access. In both cases a patch history does not need to be kept. Is that right or am I wrong here?

A simple fix would then be to state it in the README and disable the plugin hooks if lean or rawResult is detected.

[jamesholcomb]

I ended up refactoring my code by removing lean and rawResult from all write ops that use patch history.

Would it be more useful to throw an error if the lib detects either of those options?

[vinerich]

Thanks for answering!

Then I would propose throwing an error and providing a config option to disable error throwing?

[jamesholcomb]

Perhaps you could expand on the behavior of 'disable the plugin hooks'

[vinerich]

Under plugin hooks i understand the functionality of the plugin. So if thelean or rawResult option is detected we can disable the plugin functionality (configurable). So either throw error or the users "knows what he is doing" and we disable the error throwing.

[jamesholcomb]

From an end user perspective, it is hard to imagine a scenario where you would explicitly configure a collection for patch history but disable the plugin if your db writes utilize those mongoose options. Especially lean, as it it used for performance optimization. But it's hard to predict every use case, so I understand the point of view.

If you go with a "throw on error" style option, it would be prudent to enable by default.

[vinerich]

But it's hard to predict every use case, so I understand the point of view.

Isn't lean basically circumventing mongoose? It is used to gather raw results, it can be used to write if I'm not mistaken here.
So the following update operation (if any) will run through mongoose and should be supported by the patch-history-plugin again.

Just some quick thoughts on this, I'll need to consult mongoose documentation ..

If you go with a "throw on error" style option, it would be prudent to enable by default.

So the option would rather be disablePluginForLean or something like that and it would ofc default to false.
This would ensure backwards compatibility and the user should need to explicitly ignore the error.