-
Notifications
You must be signed in to change notification settings - Fork 541
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: new hooks API #3051
feat: new hooks API #3051
Conversation
0995e6e
to
c0d31c0
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.
In general looks fine to me.
Can you please also add on*Trailers
?
types/dispatcher.d.ts
Outdated
/** Invoked before response is starting to be processed */ | ||
onResponseStart?(): void; | ||
/** Invoked after status headers data has been received */ | ||
onResponseHeaders?(headers: Array<Buffer|string>, statusCode: number, statusText: string): void; |
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: remember that technically the statusText
is optional. So it should be statusText?: string
.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3051 +/- ##
==========================================
- Coverage 93.51% 93.42% -0.09%
==========================================
Files 89 89
Lines 24331 24505 +174
==========================================
+ Hits 22754 22895 +141
- Misses 1577 1610 +33 ☔ View full report in Codecov by Sentry. |
705a0d3
to
d4fb661
Compare
0302417
to
b7738a3
Compare
ae67989
to
8b139ff
Compare
Closing in favour of simpler and less complete version. #3054 |
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status