-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add pre and post invocation hooks #548
Conversation
src/WorkerChannel.ts
Outdated
private _preInvocationHooks: HookCallback[] = []; | ||
private _postInvocationHooks: HookCallback[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the interest of type safety, I would support adding separate types for pre and post invocation hook callbacks, and reflecting that here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the separate types, but I ran into too many build/lint errors if I tried to reference them here. If you can come up with a simple/clean way to do that let me know, otherwise I don't think this is worth the effort
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I gave it a try but I couldn't get them to work either, without having to use different functions to retrieve/execute each hook. I guess it's another argument for having separate functions 😋
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Hossam! Most of your comments are topics I've debated myself, so I'm glad to be ironing them out with someone else now 🙂
Met with the app insights team today. Overall looks good to them, but we need to add the function callback to the hooks api somehow. Filed #553 to address separately |
Fixes #522. Importantly, this PR does not address all possible hook-related features, but it should allow for them in the future (hopefully near future). See that issue thread for more details on that discussion.
Sample usage:
See
workerTypes/index.d.ts
for more details/docs. See here for why I picked the name I did and how I see this built-in module working in the new programming model. Ultimately, though, we could easily rename this module and do something completely different in the future.