Mongoose types incorrect for when rawResult: true is set within findOneAndUpdate middleware

[FullStackSteve]

Prerequisites

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

Mongoose version

~7.4.4

Node.js version

v16.19.1

MongoDB server version

4.0

Typescript version (if applicable)

~11.4.1

Description

Initially reported here typegoose/typegoose#886 and recommended to post here re: comment:

Incorrect types for the doc in the post findOneAndUpdate middleware when rawResult: true is set as option to the model.findOneAndUpdate(...) method

Steps to Reproduce

In typegoose:

// MongoDB: 5.0 (Docker)
// Typescript 4.9.5
import { getModelForClass, prop, post } from '@typegoose/typegoose'; // @typegoose/typegoose@11.5.1
import * as mongoose from 'mongoose'; // mongoose@7.5.4

@post<User>('findOneAndUpdate', function (doc, next) {
  // @ts-ignore
  doc.value.id = doc.value._id;  
  // @ts-ignore
  doc.value.save();
  next();
})
class User {
  @prop()
  public id: string;

  @prop()
  public username?: string;
}

const UserModel = getModelForClass(User);

async function main() {
  await mongoose.connect(`mongodb://localhost:27017/`, {
    dbName: 'verifyMASTER',
  });

  const doc = new UserModel({ username: 'user1' });

  const upsertResult = await UserModel
    .findOneAndUpdate({}, { username: 'user1' }, {
      upsert: true,
      new: true,
      rawResult: true
    })
    .exec();

  console.log(doc);

  await mongoose.disconnect();
}

main();

Roughly the mongoose equivalent:

const UserModel = mongoose.model('User', new mongoose.Schema({
  id: String,
  username: Number
}));

UserModel.post('findOneAndUpdate', function(doc, next) {
   // TYPES indicate doc.id but actual passed is doc.value.id
   doc.value.id = doc.value._id;  
   // TYPES indicate doc.save() but actual passed is doc.value.save()
   doc.value.save();
   next();
});


async function main() {
  await mongoose.connect(`mongodb://localhost:27017/`, {
    dbName: 'verifyMASTER',
  });

  const doc = new UserModel({ username: 'user1' });

  const upsertResult = await UserModel
    .findOneAndUpdate({}, { username: 'user1' }, {
      upsert: true,
      new: true,
      rawResult: true
    })
    .exec();

  console.log(doc);

  await mongoose.disconnect();
}

main();

Expected Behavior

The middleware should return the correct types for the doc parameter when the rawResult: true parameter is passed so that this change can be reflected in the typegoose library

[hasezoey]

just as a note, the mongoose code example is wrong (you try to call schema function post on the model), the corrected example would be:

const UserSchema = new mongoose.Schema({
  id: String,
  username: Number,
});

UserSchema.post('findOneAndUpdate', function (doc, next) {
  // TYPES indicate doc.id but actual passed is doc.value.id
  doc.value.id = doc.value._id;
  // TYPES indicate doc.save() but actual passed is doc.value.save()
  doc.value.save();
  next();
});

const UserModel = mongoose.model('User', UserSchema);

[IslandRhythms]

import * as mongoose from 'mongoose';

const UserSchema = new mongoose.Schema({
  id: String,
  username: String,
});

UserSchema.post('findOneAndUpdate', function (doc) {
  console.log('what is doc', doc);
  // TYPES indicate doc.id but actual passed is doc.value.id
  doc.value.id = doc.value._id.toString();
  // TYPES indicate doc.save() but actual passed is doc.value.save()
  doc.value.save();
});

const UserModel = mongoose.model('User', UserSchema);

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

  const doc = new UserModel({ username: 'user1' });

  await doc.save();
  const upsertResult = await UserModel
    .findOneAndUpdate({}, { username: 'user1' }, {
      upsert: true,
      new: true,
      rawResult: true
    })
    .exec();

  console.log(doc);
}

main();

[hasezoey]

@IslandRhythms why remove the typescript label? to my knowledge this is a issue about types

[IslandRhythms]

@hasezoey Val has told me that he only wants one label per issue so confirmed-bug overrides typescript. I do agree with you though.

[vkarpov15]

I think it is appropriate to have TypeScript label even if we also have "confirmed-bug" just to see what is a purely TypeScript issue.

[prathamVaidya]

Can I work on this issue?

[hasezoey]

you can put in a PR if you want to, i am not aware of anyone actively working on this yet

probably the easiest would be to change the hooks for which functions return a ModifyResult to be a OR of that and the original value

[prathamVaidya]

Okay! I'll check the code.

[prathamVaidya]

@hasezoey, after reviewing the Mongoose hook TypeScript declaration, I've noticed that Mongoose returns the document in hooks as the any type. (Mongoose 7.5)

The problem mentioned by @FullStackSteve is related to the types defined in the Typegoose library, specifically at this line. To resolve this issue, you have a few possible solutions:

1. You can infer the document as any in your callback function.
2. Consider updating Typegoose to return the document as any.
3. Alternatively, you can update Typegoose to return Document | ModifyResult<Document. However, if you choose this option, you'll need to check every time in your callback whether it's the original document or the modified document.

Now after the checking out the Typegoose Issue , I know you already knew about the above details. Should I implement 3rd solution in typegoose library and create a PR?

[hasezoey]

The problem mentioned by @FullStackSteve is related to the types defined in the Typegoose library, specifically at this line. To resolve this issue, you have a few possible solutions:

from what i can tell, yes this could be fixed by typegoose, but mongoose also has some types which could be changed, for example (for 7.5 & 8.0):

post<T = Query<any, any>>(method: MongooseDistinctQueryMiddleware|MongooseDistinctQueryMiddleware[], options: SchemaPostOptions, fn: PostMiddlewareFunction<T, QueryResultType<T>>): this;

could be changed to something like

post<T = Query<any, any>>(method: MongooseDistinctQueryMiddleware|MongooseDistinctQueryMiddleware[], options: SchemaPostOptions, fn: PostMiddlewareFunction<T, QueryResultType<T> | ModifyResult<QueryResultType<T>>>): this;

(would require separating out the function that return ModifyResult from MongooseDistinctQueryMiddleware

though that would likely not change the default type from "any" because of the default Query<any, any> and typescripts tendency to simplify to any instead of the types if available like any | ModifyResult<any>, but this would improve if the user provides a custom type (which is what typegoose does by default) like

UserSchema.post<
  mongoose.Query<mongoose.HydratedDocument<mongoose.InferSchemaType<typeof UserSchema>>, mongoose.InferSchemaType<typeof UserSchema>>
>

would then give the doc parameter the following type:

doc: (mongoose.Document<unknown, {}, {
    id?: string | null | undefined;
    username?: string | null | undefined;
}> & {
    id?: string | null | undefined;
    username?: string | null | undefined;
} & {
    ...;
}) | mongoose.ModifyResult<...>

(and according errors)

[prathamVaidya]

@hasezoey I got your point. This can be useful if user provides a custom type.
However, I'd like you to clarify the part where you mentioned:

(would require separating out the function that return ModifyResult from MongooseDistinctQueryMiddleware

Is this related to separating the ModifyResult<> type if rawResult is found in query options because query options are not accessible in hooks.

Also, anyways user will have to add a manual check for "value" to differentiate between the ModifyResult<> and Document in the callback function. Is there anyway that can be avoided or improved?

From what I gather, my current understanding is that I need to add ModifyResult in the post hooks that supports rawResult and create a PR for the master branch.

[hasezoey]

Is this related to separating the ModifyResult<> type if rawResult is found in query options because query options are not accessible in hooks.

i dont quite know what you are asking, i mean separating out things like find*And* which could return a ModifyResult from functions that do not (like count), i am not sure exactly which function do and dont return that and may require double-checking with the runtime code to verify

Is there anyway that can be avoided or improved?

to my knowledge this cannot be avoided, and the only known ways of dealing with this are:

• cast the value to what you want (if you are sure you wont ever use the other version)
• assert the value (assuming the assert function used actually has the proper types, see assertion in typegoose's utils)
• use if-blocks like if ("value" in doc) { ModifyResult }

the last 2 options (assert and if-blocks) could be slightly improved by adding a helper function to detect if it is actually a ModifyResult instead of something else, that would need to be a bit more involved, because the passed-in object (read document) could also have its own value defined in its schema

[prathamVaidya]

@hasezoey Apologies for the delayed response; I was occupied for a while. I've identified the middlewares that support includeResultMetadata and added separate types to fix the Issue in v8.0.

Check the PR here.

Let me know if its acceptable or I need to fix something else.