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

fix: Add generic to plugin schema definition #9968

Merged

Conversation

emiljanitzek
Copy link
Contributor

Summary

Update the typescript definition for plugin() on Schema to add generic support. Currently, the plugin() complains when trying to add a plugin with custom Document/Model definitions.

Examples

const myDocumentSchema = new Schema<MyDocument, MyModel>({ name: String });

export default function myPlugin(mySchema: Schema<MyDocument, MyModel>): void {
    mySchema.add({
        plugin: Boolean
    })
}

myDocumentSchema.plugin(myPlugin); // <-- this does not work since Schema in plugin does not support generics

index.d.ts Outdated
@@ -1123,7 +1123,7 @@ declare module 'mongoose' {
pathType(path: string): string;

/** Registers a plugin for this schema. */
plugin(fn: (schema: Schema, opts?: any) => void, opts?: any): this;
plugin<T extends Document = DocType, L extends Model<T> = M>(fn: (schema: Schema<T, L, SchemaDefinitionType>, opts?: any) => void, opts?: any): this;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are the generics necessary? Adding schema: Schema<DocType, Model<DocType>, SchemaDefinitionType> should be sufficient for this case, shouldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, it would be sufficient for the case above. I was following the same pattern as found with post/pre methods just below. I can update according to your suggestion if you would prefer that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please, I'd rather avoid adding unnecessary generic params if possible.

@emiljanitzek emiljanitzek force-pushed the feature/plugin-schema-type branch from c6727a5 to 8788047 Compare March 3, 2021 07:48
@emiljanitzek emiljanitzek force-pushed the feature/plugin-schema-type branch from 8788047 to b24b917 Compare March 3, 2021 07:53
@emiljanitzek emiljanitzek requested a review from vkarpov15 March 3, 2021 07:55
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@vkarpov15 vkarpov15 added this to the 5.11.19 milestone Mar 5, 2021
@vkarpov15 vkarpov15 added the typescript Types or Types-test related issue / Pull Request label Mar 5, 2021
@vkarpov15 vkarpov15 merged commit 01ffe2f into Automattic:master Mar 5, 2021
This was referenced Mar 5, 2021
This was referenced Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants