Skip to content

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

/** 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