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!: introduce declarative function signatures #347
Conversation
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.
Great, thanks for the PR! I've left review and comments.
- Have we thought of the design of the interface a bit more? We might want to change the developer interface of the function register.
- Could we have a more detailed PR description? What are the user-facing changes, and what is the newly exposed interface? What if developers mix their existing functions with declarative functions?
- We probably want to include documentation changes. That can be in this PR or a following PR.
src/registration_container.ts
Outdated
import {HttpFunction, CloudEventFunction, HandlerFunction} from './functions'; | ||
import {SignatureType} from './types'; | ||
|
||
interface RegisteredFunction { |
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.
nit: comment
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.
This interface isn't exported. It is a private implementation detail of this module that is pretty self explanatory. I prefer not to add comments for these types of things.
src/registration_container.ts
Outdated
signatureType: SignatureType, | ||
userFunction: HandlerFunction | ||
): void => { | ||
registrationContainer.set(functionName, { |
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 don't think we defined the contract/interface yet.
But this implementation seems to assume that a single functionName
can only map to 1 signature type and 1 userFunction.
Is that right and is that subject to change?
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.
That's right, and I don't think it is subject to change. I can't imagine a use case that would motivate more flexibility. I was even considering adding validation logic that throws an exception if a function with the same name is registered more than once.
978b26c
to
83fcd49
Compare
This commit introduces an API that allows developers to explictly register their functions against the framework so that they can declaratively configure thier signature type.
83fcd49
to
3329ddc
Compare
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.
lgtm :)
): Promise<HandlerFunction | null> { | ||
functionTarget: string, | ||
signatureType: SignatureType | ||
): Promise<{ |
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.
Optional: Since we're adding a 3rd param, we might want to use an object rather than ordered list.
export async function getUserFunction({
codeLocation,
functionTarget,
signatureType,
} ... )
That way, we could destructure when calling / using this function.
This commit introduces an API that allows developers to explicitly register their functions against the framework so that they can declaratively configure the signature type in their application source code:
When functions are registered using this API, the configured signature types take precedence over the
--signature-type
flag andSIGNATURE_TYPE
environment variables.When initialized the functions framework:
--source
flag orFUNCTION_SOURCE
env var--target
flag orFUNCTION_TARGET
env var--source
module that matches the target name specified var the--target
flag orFUNCTION_TARGET
env var