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

[Hooks] Evolving the types #665

Closed
aaronpowell opened this issue Mar 8, 2023 · 3 comments
Closed

[Hooks] Evolving the types #665

aaronpowell opened this issue Mar 8, 2023 · 3 comments

Comments

@aaronpowell
Copy link

Parent topic - #664

Looking at the types as defined here

function registerHook(hookName: 'preInvocation', callback: PreInvocationCallback): Disposable;
function registerHook(hookName: 'postInvocation', callback: PostInvocationCallback): Disposable;
function registerHook(hookName: 'appStart', callback: AppStartCallback): Disposable;
function registerHook(hookName: 'appTerminate', callback: AppTerminateCallback): Disposable;
function registerHook(hookName: string, callback: HookCallback): Disposable;
type HookCallback = (context: HookContext) => void | Promise<void>;
type PreInvocationCallback = (context: PreInvocationContext) => void | Promise<void>;
type PostInvocationCallback = (context: PostInvocationContext) => void | Promise<void>;
type AppStartCallback = (context: AppStartContext) => void | Promise<void>;
type AppTerminateCallback = (context: AppTerminateContext) => void | Promise<void>;

There isn't much in common with them, the type of hook to be registered are strings in the overload and the types for each callback don't have a common base type. As a result, trying to create a generic middleware function in the library project can be difficult without a lot of overloads there to ensure that the type system is happy.

Using an enum or union for the name of the hook would make it simpler to have other places within the code be aware of valid hook names.

For the callbacks, it'd be helpful if they inherited a base callback type as it can make for simpler type signatures elsewhere where you might want to create an abstraction layer over the hooks (again, something that could be exposed in the new programming model).

@ejizba
Copy link
Contributor

ejizba commented Apr 20, 2023

Discussed this one a bit during today's sync. We don't want people using the "@azure/functions-core" api directly. It's clunky to use, it's slow for us to roll out changes, and we have to be super careful about backwards compatibility. There's quite a few hooks requests (more than we have time for) and we want to prioritize the work on the npm package. Closing this issue in favor of Azure/azure-functions-nodejs-library#7

@aaronpowell
Copy link
Author

I think that for any of the work in the library to be done, this will need to be tackled as the library package will need these types and at present they are inflexible and make creating abstractions difficult.

@ejizba
Copy link
Contributor

ejizba commented Apr 21, 2023

Discussed a bit offline. Current plan is to implement all of this stuff in the library package, not the worker. I don't think it'll be too inflexible/difficult, but we can always revisit if @hossam-nasr finds otherwise when working on the prototype.

Also when I say "library" package I'm talking specifically about @azure/functions. Aaron brought up that there could be other libraries that want to reference @azure/functions-core instead of @azure/functions. We want to see what people think of the support in @azure/functions before we go down that road, though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants