Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing Typescript detail on Mongoose-specific options for updateOne, updateMany, etc. #14378

Closed
2 tasks done
sderrow opened this issue Feb 26, 2024 · 3 comments · Fixed by #14382
Closed
2 tasks done
Labels
discussion If you have any thoughts or comments on this issue, please share them!

Comments

@sderrow
Copy link
Contributor

sderrow commented Feb 26, 2024

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.2.0

Node.js version

18.18.0

MongoDB server version

7.0

Typescript version (if applicable)

4.5.2

Description

In #14342 , the MongooseQueryOptions type was edited to include & { [other: string]: any } in order to support arbitrary options. Arbitrary options are fine, but the specific implementation broke Typescript support for all of the official Mongoose option properties, in the sense that there's no more auto-completion or in-line JSDoc displayed when you start typing something like "runValidators".

This is caused by the use of the Omit Typescript utility in the type for options in updateOne, updateMany, and a few others. Since now Omit is being called on a type that has [other: string]: any in it, that particular weak "other" property essentially wipes out all the official properties (I think due to the underlying implementation of Exclude).

Steps to Reproduce

export const queryOptionTypeTest = async (): Promise<void> => {
  type Test = {
    _id: mongoose.Types.ObjectId;

    testProp: string;

    createdAt: number;
    updatedAt: number;
  };

  const schema = new mongoose.Schema<Test>(
    {
      testProp: { type: String },
      createdAt: { type: Number },
      updatedAt: { type: Number },
    },
    {
      timestamps: { currentTime: () => new Date().valueOf() },
    }
  );

  const M = mongoose.model<Test>("Test", schema);

  // Typescript offers no help for the `timestamps` or `runValidators` options below.
  // It treats them as if they were arbitrary options instead of formal ones with known meanings
  await M.updateMany({}, { testProp: "hello" }, { timestamps: false, runValidators: true }).exec();
};

Here's a screenshot of my IDE with the bug and trying to auto-complete the "timestamps" options. You can see it only shows "maxTimeMS" instead.
updateOptionsBugWithoutFix

Expected Behavior

Here's what I would expect my IDE to show me. You can see it includes auto-complete and helpful documentation for "timestamps" now.
updateOptionsBugWithFix

@FaizBShah
Copy link
Contributor

Hi @sderrow , I tried to solve this issue in a different manner by not introducing new base types. Basically, replaced Omit with a custom util ExcludeKeys which fixes the issue caused by Omit. Do review the PR and tell whether it looks ok to you? #14382

@sderrow
Copy link
Contributor Author

sderrow commented Feb 26, 2024

@FaizBShah thanks! Your implementation looks good to me. I don't have a strong opinion on implementation. The one additional upside to creating formal types for the base and update query options like I did in #14379 is being able to re-use them instead of having to redeclare the same exclusion of keys across multiple methods. A bit more DRY. But defer to core devs on this one!

@FaizBShah
Copy link
Contributor

@sderrow Yes, agree with the re-use factor and DRY advantage of the base formal types. I also side with deferring this to the core devs!!

@IslandRhythms IslandRhythms added the discussion If you have any thoughts or comments on this issue, please share them! label Feb 26, 2024
vkarpov15 added a commit to FaizBShah/mongoose that referenced this issue Feb 26, 2024
vkarpov15 added a commit that referenced this issue Feb 26, 2024
fix: missing typescript details on options params of updateMany, updateOne, etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion If you have any thoughts or comments on this issue, please share them!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants