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(types): make schema.plugin() more flexible for schemas that don't define any generics #12486

Merged
merged 3 commits into from Sep 30, 2022

Conversation

vkarpov15
Copy link
Collaborator

Fix #12454
Re: #12139

Summary

I think just switching to any should be enough to fix #12454, it doesn't break any of the tests from #12139. Any thoughts @emiljanitzek ?

Examples

@emiljanitzek
Copy link
Contributor

This would make the plugin function type a lot more loose and you would not get the same type safety. So I would prefer it not to change. I would say that the problem is that the Schema class is too strictly typed by default. I haven't tested this but I think changing the Schema class generics should have the same effect without changing the plugin interface.

export class Schema<
    EnforcedDocType = any,
    M = Model<EnforcedDocType, any, any, any>,
    TInstanceMethods = any,
    TQueryHelpers = any,
    TVirtuals = any,
    TStaticMethods = any,
    TPathTypeKey extends TypeKeyBaseType = DefaultTypeKey,
    DocType extends ObtainDocumentType<DocType, EnforcedDocType, TPathTypeKey> = ObtainDocumentType<any, EnforcedDocType, TPathTypeKey>>
    extends events.EventEmitter {

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

LGTM, somehow i missed this as a possible solution when making #12458

@vkarpov15
Copy link
Collaborator Author

The unfortunate issue is that changing TInstanceMethods = {} to TInstanceMethods = any causes a lot more problems with schema type inference, and those problems will be harder to fix. The suggestion to make schema generics default to any is an interesting one, we'll have to consider it further. But, for now, we should loosen the types up a bit to avoid potentially breaking changes.

@vkarpov15 vkarpov15 merged commit 06d00fa into master Sep 30, 2022
@hasezoey hasezoey deleted the vkarpov15/gh-12454 branch September 30, 2022 16:13
@hasezoey hasezoey added this to the 6.6.3 milestone Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to correctly type custom plugins? Also unable to register previously working plugins since v6.5.2
3 participants