fix(types): better typescript support for schema plugins #12139
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emiljanitzek
force-pushed
the
feature/plugin-types
branch
from
02b7856
to
cb93470
Compare
vkarpov15
requested changes
Jul 25, 2022
types/index.d.ts
Outdated
| @@ -291,15 +296,15 @@ declare module 'mongoose' { | |||
| set<K extends keyof SchemaOptions>(key: K, value: SchemaOptions[K], _tags?: any): this; | |||
|
|
|||
| /** Adds static "class" methods to Models compiled from this schema. */ | |||
| static(name: string, fn: (this: M, ...args: any[]) => any): this; | |||
| static(obj: { [name: string]: (this: M, ...args: any[]) => any }): this; | |||
| static(name: keyof TStaticMethods, fn: (this: M, ...args: any[]) => any): this; | |||
There was a problem hiding this comment.
This change is a bit risky, because it will break compile for anyone that uses static() without setting TStaticMethods. Can we add a method overload that still allows calling static() with an arbitrary string?
types/index.d.ts
Outdated
|
|
||
| /** Creates a virtual type with the given name. */ | ||
| virtual<T = HydratedDocument<DocType, TInstanceMethods>>( | ||
| name: string, | ||
| name: keyof TVirtuals, |
There was a problem hiding this comment.
Same with above. This change is a bit risky.
emiljanitzek
force-pushed
the
feature/plugin-types
branch
from
cb93470
to
d89a7b6
Compare
|
Thanks for the feedback @vkarpov15. I've updated the types and added test for this as well. |
vkarpov15
approved these changes
Jul 26, 2022
emiljanitzek
force-pushed
the
feature/plugin-types
branch
from
d89a7b6
to
ca3444d
Compare
|
Looks like there were some merge conflict issues, because tests are failing now. Can you please patch that up @emiljanitzek ? I took a look and it looks like this change doesn't cause any significant performance degradation. |
emiljanitzek
force-pushed
the
feature/plugin-types
branch
3 times, most recently
from
7e5ac5d
to
d000b26
Compare
emiljanitzek
force-pushed
the
feature/plugin-types
branch
from
d000b26
to
faa6422
Compare
|
@vkarpov15 rebased and fixed the conflicts. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Found more problems when working with typescript and plugins. Since the Schema type sets default values (empty object) for
TInstanceMethods,TQueryHelpersandTStaticMethodswe need to be able to pass along these types when trying to create query helpers/virtuals/methods/statics.This PR also contains the fix for Schema and model mismatch. Will rebase this if the other PR gets accepted.
Examples
Added
plugin.tests.tsfor examples