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

TypeScript: remove support for document interfaces that extends Document #11615

Closed
vkarpov15 opened this issue Apr 2, 2022 · 3 comments
Closed
Labels
backwards-breaking typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@vkarpov15
Copy link
Collaborator

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

Neither

What is the current behavior?

We've been recommending people not use extends Document for the document interface generic param to Schema, Model, etc.

// Do this:
interface User {
  name: string;
  email: string;
  // Use `Types.ObjectId` in document interface...
  organization: Types.ObjectId;
}

const schema = new Schema<User>({
  name: { type: String, required: true },
  email: { type: String, required: true },
  // And `Schema.Types.ObjectId` in the schema definition.
  organization: { type: Schema.Types.ObjectId, ref: 'Organization' }
});

// Not this:
interface User extends Document {
  name: string;
  email: string;
  // Use `Types.ObjectId` in document interface...
  organization: Types.ObjectId;
}

See: https://mongoosejs.com/docs/typescript.html#using-extends-document

This makes our TypeScript types considerably more complex, and also adds a non-trivial amount of performance overhead because of all the places where we need to Omit<> document keys.

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

What is the expected behavior?

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

@tristanguerin
Copy link

Hi @vkarpov15,

Thanks for the notice !

Sometime we need the document version in our applications.
Do you have any counter-argument of implementing the following UserDocument declaration ?

// Do this:
interface User {
  name: string;
  email: string;
  // Use `Types.ObjectId` in document interface...
  organization: Types.ObjectId;
}

const schema = new Schema<User>({
  name: { type: String, required: true },
  email: { type: String, required: true },
  // And `Schema.Types.ObjectId` in the schema definition.
  organization: { type: Schema.Types.ObjectId, ref: 'Organization' }
});

// Document type to match correct type after using findOne() ... (no lean)
type UserDocument = User & Document;

I think it wouldn't impact performances but we've had severe performances issues (and sometime still have), so it'd be great to have confirmation.

@vkarpov15
Copy link
Collaborator Author

@tristanguerin that approach works. We recommend doing Document & User instead, but those should be equivalent in your case.

What sort of performance issues are you seeing and what version of Mongoose are you using?

@tristanguerin
Copy link

@vkarpov15
Sometime VSCode is slow to display autocompletion on documents (hardly reproductible, might come from my setup).
We're using version 6.0.14.

Thank you for your answer and your recommendations.

vkarpov15 added a commit that referenced this issue Jan 18, 2023
vkarpov15 added a commit that referenced this issue Jan 18, 2023
…ather than separate TVirtuals and TInstanceMethods

Re: #11615
vkarpov15 added a commit that referenced this issue Jan 19, 2023
vkarpov15 added a commit that referenced this issue Feb 4, 2023
Drop support for document interfaces that `extends Document`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-breaking typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

No branches or pull requests

2 participants