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

feat(schema+types): add { errorHandler: true } option to Schema post() for better TypeScript support #12723

Merged
merged 8 commits into from
Dec 4, 2022

Conversation

vkarpov15
Copy link
Collaborator

@vkarpov15 vkarpov15 commented Nov 25, 2022

Fix #12583

Summary

Requires mongoosejs/kareem#34 . You can now do schema.post('save', { errorHandler: true }, async function(err) { ... }). This means you can use error handling middleware without explicitly listing out arguments, and also makes TypeScript better able to infer the type of params to error handling middleware.

Examples

@vkarpov15 vkarpov15 added this to the 6.8 milestone Nov 25, 2022
@hasezoey
Copy link
Collaborator

This PR maybe should be changed to a "Draft" (WIP) PR, so that it can not be accidentally merged (even if tests are failing)

@vkarpov15 vkarpov15 marked this pull request as draft November 29, 2022 17:16
@vkarpov15
Copy link
Collaborator Author

@hasezoey good suggestion. But can you please review this PR and mongoosejs/kareem#34 anyway? This PR is ready to go, just hard to test because it requires a PR from a separate repo. Tests pass locally, I just want to make sure the API design makes sense.

@hasezoey
Copy link
Collaborator

But can you please review this PR

will try, when the required lower PR is merged and this PR is updated to actually use that package instead of relative path

types/middlewares.d.ts Outdated Show resolved Hide resolved
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

and "finally" kareem shipped a new version which cut the size in less than half (thanks to not including useless files like tests anymore)
for reference, 2.4.1 was 109 kB and 2.5.0 is 38.9 kB (bundle size as reported by npm)

@vkarpov15 vkarpov15 marked this pull request as ready for review December 4, 2022 21:05
@vkarpov15 vkarpov15 merged commit efed0a4 into 6.8 Dec 4, 2022
@vkarpov15 vkarpov15 deleted the vkarpov15/gh-12595 branch December 4, 2022 21:06
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.

2 participants