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

Issues when migrating from 5.12.3 to 6.8.0 #12806

Closed
1 task done
pavlos163 opened this issue Dec 16, 2022 · 6 comments · Fixed by #12912
Closed
1 task done

Issues when migrating from 5.12.3 to 6.8.0 #12806

pavlos163 opened this issue Dec 16, 2022 · 6 comments · Fixed by #12912
Assignees
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@pavlos163
Copy link

pavlos163 commented Dec 16, 2022

Prerequisites

  • I have written a descriptive issue title

Mongoose version

6.8.0

Node.js version

16.10.0

MongoDB version

5.0.14

Operating system

None

Operating system version (i.e. 20.04, 11.3, 10)

No response

Issue

I've completed the migration of my code from using Mongoose 5.12.3 to using 6.8.0.

I've followed the guidelines in this guide, and everything has worked as expected except from a couple of things:

Usage of this in virtual functions when also using discriminators.

I have a main schema that contains some basic values and two schemas that inherit from it. Let's say, for the sake of this issue, that I have a Payments collection, and two types of Payments that inherit from the main Payment schema, BankPayment and CashPayment.

Up until this point, I would structure this as follows:

export const PaymentCategoryArray = [
    "CashPayment",
    "BankPayment",
] as const;
export type PaymentCategoryType = typeof PaymentCategoryArray[number];

interface PaymentInterface {
    owner: mongoose.Types.ObjectId;
    category: PaymentCategoryType;
}

interface BankPaymentInterface extends PaymentInterface {
    bankReference: string;
}

interface CashPaymentInterface extends PaymentInterface {
    cashAmount: number
}

export interface PaymentDocument extends PaymentInterface, Document {}

export interface BankPaymentDocument extends BankPaymentInterface, PaymentDocument {}

export interface CashPaymentDocument extends CashPaymentInterface, PaymentDocument {}

/**
 * SCHEMAS
 */
const paymentSchema = new mongoose.Schema(
    {
        owner: {
            type: mongoose.Schema.Types.ObjectId,
            ref: "User",
            required: true
        }
    },
    { discriminatorKey: "category" }
);
const bankPaymentSchema = new mongoose.Schema(
    {
        bankReference: { type: String, required: true },
    }
);
const cashPaymentSchema = new mongoose.Schema(
    {
        cashAmount: { type: Number }
    }
);

export const Payment = mongoose.model<PaymentDocument>("Payment", paymentSchema);
export const BankPayment = Payment.discriminator<BankPaymentDocument>("BankPayment", bankPaymentSchema);
export const CashPayment = Payment.discriminator<CashPaymentDocument>(
    "CashPayment",
    cashPaymentSchema
);

And up until this point, I could implement virtual methods such as this:

bankPaymentSchema.virtual("displayOwner").get(function (): string {
    return this.owner.toString();
});

However, when we migrated to 6.8.0, this started complaining with the following error:

TS2339: Property 'owner' does not exist on type 'Document  & { bankReference: string; } & { _id: ObjectId; }'.

And it seems like I am forced to cast the above like so:

bankPaymentSchema.virtual("displayOwner").get(function (): string {
    const document = this as PaymentInterface;
    return document.owner;
});

But even this fails with:

TS2352: Conversion of type 'Document<unknown, any, { bankReference: string; }> & { bankReference: string; } & { _id: ObjectId; }' to type 'PaymentInterface' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.   Type 'Document<unknown, any, { bankReference: string; }> & { bankReference: string; } & { _id: ObjectId; }' is missing the following properties from type 'PaymentInterface': owner, category

I am trying to understand what this is in the context of a virtual method implementation such as the above and how I can work around this.

Population of _id in all subdocuments

It seems like from the moment I migrated to 6.8.0, I have to specify _id: false in all the object types in my schemas. Is there a way to default to this behaviour? I don't have any use cases I'd want a _id field in an object in my schema.

@pavlos163 pavlos163 added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Dec 16, 2022
@IslandRhythms
Copy link
Collaborator

I'm not sure I understand your question. The docs are very clear

You can also overwrite Mongoose's default _id with your own _id. Just be careful: Mongoose will refuse to save a document that doesn't have an _id, so you're responsible for setting _id if you define your own _id path.

What is a Subdocument?
Subdocuments are similar to normal documents. Nested schemas can have middleware, custom validation logic, virtuals, and any other feature top-level schemas can use. The major difference is that subdocuments are not saved individually, they are saved whenever their top-level parent document is saved.

Are you requesting a feature to disable _id globally?

@pavlos163
Copy link
Author

Well, kind of, part of my second question is if if there is a way to disable _id for subdocuments globally. If there is any other work around or insight into this then that is also welcome. Another part of the question is why this is the case only after migrating to 6.8.0, as the migration docs did not mention such change will be needed.

(all that of course is only relevant to the second question and not the first)

@IslandRhythms
Copy link
Collaborator

Well, kind of, part of my second question is if if there is a way to disable _id for subdocuments globally.

Currently no.

this is the case only after migrating to 6.8.0

Why what is the case? Not having the option to disable _id for subdocs globally or having to set _id:false on all your object types. If its the latter, that is a question for @vkarpov15

@hasezoey
Copy link
Collaborator

as a note, you can globally disable _id, just note that this does not disable _id for root documents, only subdocuments
Note: if used in typescript you will need the as any because the modified property is not yet in the types
Note: if doing .create({ nest: {} }) and have no default's in the subdocument, then mongoose will treat it as a empty value not and save it
Note: this also affects Subdocument arrays, because the arrays extend from Subdocument

// "as any" casting is currently required because "defaultOptions" does not exist in the mongoose types
(mongoose.Schema.Types.Subdocument as any).defaultOptions = { ...(mongoose.Schema.Types.Subdocument as any).defaultOptions, _id: false };

put the above code before creating / importing any schemas


TS2339: Property 'owner' does not exist on type 'Document & { bankReference: string; } & { _id: ObjectId; }'.

this error is probably because you are trying to use property owner on a virtual for bankPaymentSchema, but the type and schema for bankPaymentSchema does not extend from paymentSchema yet, only BankPayment does because discriminator by default merges the base schema into the discriminated schema

the best solution would probably to be to set the schema type with const bankPaymentSchema = new mongoose.Schema<BankPaymentDocument>(


btw, it is not recommended to extend from Document as done in export interface PaymentDocument extends PaymentInterface, Document {} (though i cannot find the reference on where i read that anymore)

personally i recommend using a simple wrapper type (mostly copied from typegoose):

type DocumentType<T, QueryHelpers = {}> = (T extends {
    _id: unknown;
} ? mongoose.Document<T['_id'], QueryHelpers, T> : mongoose.Document<any, QueryHelpers, T>) & T;

// which can be used like

type PaymentDocument = DocumentType<PaymentInterface>;
type BankPaymentDocument = DocumentType<BankPaymentInterface>;
type CashPaymentDocument = DocumentType<CashPaymentInterface>;

@hasezoey hasezoey added the typescript Types or Types-test related issue / Pull Request label Dec 20, 2022
@vkarpov15 vkarpov15 added this to the 6.8.4 milestone Dec 29, 2022
@github-actions
Copy link

This issue is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the Stale label Jan 13, 2023
@vkarpov15 vkarpov15 removed the Stale label Jan 14, 2023
@vkarpov15
Copy link
Collaborator

In the following code:

bankPaymentSchema.virtual("displayOwner").get(function (): string {
    return this.owner.toString();
});

in Mongoose 5.12.3, this was any. In Mongoose 6.8, Mongoose sets the type of this to whatever the document type is. In your case, Mongoose schema type inference just picks up whatever is defined in your schema, so it thinks the only field on a BankPayment is bankReference.

Here's a couple of workarounds:

  1. Override Mongoose's schema type inference and tell Mongoose that there are additional fields on bankPaymentSchema that it doesn't know about, as follows. This would likely be the best approach.
const bankPaymentSchema = new Schema<PaymentInterface & BankPaymentInterface>(
    {
        bankReference: { type: String, required: true },
    }
);
  1. If ☝️ doesn't work, the lowest friction approach is to overwrite the type of this in the virtual using generics:
bankPaymentSchema.virtual<BankPaymentDocument>("displayOwner").get(function (): string {
    return this.owner.toString();
});

@vkarpov15 vkarpov15 added docs This issue is due to a mistake or omission in the mongoosejs.com documentation and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Jan 16, 2023
vkarpov15 added a commit that referenced this issue Jan 17, 2023
docs(typescript): add notes about virtual context to Mongoose 6 migration and TypeScript virtuals docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation typescript Types or Types-test related issue / Pull Request
Projects
None yet
4 participants