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

Allow async preHandler hooks on RouteShorthandOptions #5152

Closed
wants to merge 1 commit into from

Conversation

SlyryD
Copy link

@SlyryD SlyryD commented Nov 9, 2023

Description

In response to issue #5086, where I mentioned that the hookRunner function can handle async preHandler hooks, I added types to route.d.ts to allow async preHandler hooks. This change makes it clear that you want either a callback-based hook that returns void or an async hook that returns a Promise -- using an async hook with a callback is problematic and not handled properly by hookRunner. This change can probably be applied to the other hooks in RouteShorthandOptions, but I scoped it just to the preHandler hook, which I tested locally. In fact, there are already tests in route.test-d.ts that validate async hooks.

Checklist

@github-actions github-actions bot added the typescript TypeScript related label Nov 9, 2023
@SlyryD
Copy link
Author

SlyryD commented Nov 10, 2023

TypeScript isn't able to infer the parameter types with my changes. I'll mark this as Draft for now.

@SlyryD SlyryD marked this pull request as draft November 10, 2023 01:09
@bienzaaron
Copy link
Contributor

The same type update was attempted in #4655, but then reverted due to this issue #4705 (same thing you're describing)

I opened #5147 which allows async hooks without breaking the inference on parameters

@SlyryD
Copy link
Author

SlyryD commented Nov 13, 2023

Closed in favor of #5147

@SlyryD SlyryD closed this Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript TypeScript related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants