mongoose: LeanDocument Type doesn't include _id or id

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

What is the current behavior? I believe that the types defined here are incorrect: https://github.com/Automattic/mongoose/blob/c22152c5a95c5ce9ed688d96b37b05c7df6be1ec/types/index.d.ts#L2261-L2267 This says that LeanDocuments (that is documents returned from .toObject() or when using the lean option don’t have the _id or id fields. I don’t think this is correct.

This issue only presented itself in version 6.3.2 and wasn’t present in 6.3.1

If the current behavior is a bug, please provide the steps to reproduce.

import mongoose from 'mongoose'

async function test() {
  const thingSchema = new mongoose.Schema({
    name: mongoose.Schema.Types.String,
  });

  const ThingModel = mongoose.model('Thing', thingSchema);

  // TypeScript will complain here that `_id` doesn't exist
  const { _id, ...thing1 } = (await ThingModel.create({name: 'Thing 1'})).toObject();

  // if I ignore the above warning and this log happens, _id is a valid ObjectId.
  console.log({_id, thing1});
}

What is the expected behavior?

_id and id are present on LeanDocuments (I think…) What are the versions of Node.js, Mongoose and MongoDB you are using? Note that “latest” is not a version.

Node.js: v16.15.0 TypeScript: v4.6.2 mongoose: v6.3.2

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 43 (35 by maintainers)

Most upvoted comments

This is a clear regression. This is comparable to when Apple had antenna issues with their iphones, if you hold the IPhones in your hand and they basically said, that it is the consumers fault, because the consumer hold the iPhone “wrong”…

This is not acceptable.

@mohammad0-0ahmad We need to fix this. Any idea how we can fix that in a timely manner?

Uzlopak on May 3, 2022

I went to sleep, woke up and saw the notifications and I thought: “Oh I hope the devs did not get frustrated that one and not the other PR was merged”. First of all I thank you both, @taxilian and @mohammad0-0ahmad and others . I know that it could been frustrating but I think that you both had a respectful and valuable discussion why doing this or that.

And yeah, it is kind of pedantic/neurological disorder to type complex code 😃. I have that too. I always try to make the typings strict as possible and correct as possible. People love and hate it 🗡️

Anyway. I think that we have to rethink the whole typings. I actually also dont understand why we have LeanDocument in the first place as it should be a POJO, which shape is based on the projection and if the lean flag or function was set. So I actually wonder if the following would make sense:

The first thing is to get the automatic typing from mohammad0 getting properly reviewed and merged. I invite you, @taxilian to also review it. I will probably also review it and try and some test cases.

But actually what should happen is, that when we create a query e.g. findOne, it should return Document<T> if not lean option was set to true or lean() was called and it should return T, were T is in both cases the resulting type of the potential fields/projection setting.

Uzlopak on May 5, 2022

It’s totally fine, it doesn’t matter which one will be merged the important thing it to get the best possible result 😃

mohammad0-0ahmad on May 5, 2022

I respectfully disagree. We dont break user space without making a major release. Now we force devs to patch their schemas or write ts-ignore. Also it would mean that the typings do not represent the javascript reality.

@mohammad0-0ahmad Would the autotyping PR fix this implicitly?

I’ve created new PR instead.😉

mohammad0-0ahmad on May 4, 2022