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!: update to latest version of clouevents sdk #402
Conversation
c7aa8f9
to
1cf9eed
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.
Awesome! This is the ideal experience I've been looking for with event types.
Had a couple comments, mainly for making sure this is tested.
e16b6d9
to
dfdfb92
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.
Looks pretty good. Are we able to test with an unparameterized CloudEvent
to preserve the existing test?
LGTM after requested changes.
This commit updates to the latest CESDK which provides a generic interface for CloudEvents. This is an additive, non-breaking change to our registration API
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. I think there might be a push that is missing.
dfdfb92
to
2109890
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!
@@ -39,7 +39,7 @@ const TEST_EXTENSIONS = { | |||
describe('CloudEvent Function', () => { | |||
let clock: sinon.SinonFakeTimers; | |||
|
|||
let receivedCloudEvent: functions.CloudEvent | null; | |||
let receivedCloudEvent: functions.CloudEvent<unknown> | null; |
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.
It would be a little funky requiring a unknown
here. Question:
functions.CloudEvent
(can we do this?)functions.CloudEvent<unknown>
| CloudEventFunction<T> | ||
| CloudEventFunctionWithCallback<T>; |
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.
Is it possible for a developer to write a CloudEventFunction without a parameterized type?
Perhaps it is not possible.
export type HandlerFunction<T = unknown> =
| HttpFunction
| EventFunction
| EventFunctionWithCallback
| CloudEventFunction<T>
| CloudEventFunctionWithCallback<T>
| CloudEventFunction
| CloudEventFunctionWithCallback;
This commit updates to the latest CESDK which provides a generic interface for CloudEvents. This is an additive, non-breaking change to our registration API:
providing a template type on the registration function:
providing a template type on the argument annotation:
not providing a template is still allowed: