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): correctly infer this when using pre('updateOne') with { document: true, query: false } #12778

Merged
merged 6 commits into from Dec 19, 2022

Conversation

vkarpov15
Copy link
Collaborator

Fix #11257

Summary

pre('updateOne') currently infers this is a document, rather than a query by default.

Examples

@hasezoey
Copy link
Collaborator

hasezoey commented Dec 8, 2022

ts-benchmark is failing with this PR, is this something that can be fixed without removing the types again?

@vkarpov15
Copy link
Collaborator Author

@hasezoey that's just due to TS slowdowns creeping up on us, and this change is just the straw that broke the camel's back. This PR increased instantiations from around 99k to just over 100k.

I think the PR that started the TS performance degradation creep was this one: 76eaadd . Here's TS benchmark after: https://github.com/Automattic/mongoose/actions/runs/3231069336/jobs/5329705376 and previous TS benchmark run: https://github.com/Automattic/mongoose/actions/runs/3212307397/jobs/5251060253 . We'll investigate and see if we can get rid of some of the perf overhead of #12536

@hasezoey
Copy link
Collaborator

hasezoey commented Dec 8, 2022

i guess we just need / can only increase the threshold now, the types are just getting more complex to actually represent what is going on at runtime.
(i am not a typescript wizard, and i can barely get the types to be correct, so i have no clue about optimizing them while still being correct)

@vkarpov15 vkarpov15 merged commit 5b62093 into master Dec 19, 2022
@hasezoey hasezoey deleted the vkarpov15/gh-11257 branch December 20, 2022 12:24
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.

schema.pre('save' for document not working since 6.1.5
2 participants