feat: add before and after request methods to base service #961
Conversation
There was a problem hiding this comment.
Hey @lukealvoeiro , I had a few questions, but this is a great PR, thanks for submitting it!
src/generate-services.ts
Outdated
| const additionalMethods = []; | ||
| const is$TypePresent = options.outputTypeAnnotations || options.outputTypeRegistry; | ||
| if (options.outputAfterResponse) { | ||
| additionalMethods.push(code`afterResponse?(response: { ${is$TypePresent ? "$type: string" : ""} }): void;`); |
There was a problem hiding this comment.
@stephenh Since the request / response type is truly dynamic, I was hoping to get your thoughts on this call signature. Options I can think of are the following:
// OPTION 1: (Current)
beforeRequest: (request: {}): void; // or
beforeRequest: (request: { $type: string }): void;
// OPTION 2: using a index signature
beforeRequest: (request: {[property: string]: any)}: void;
// OPTION 3: using any or object 😢
beforeRequest: (request: object): void;Note that I would be adding an inline comment in ts-proto and to the generated file to point this out.
There was a problem hiding this comment.
Hm, yeah, I dunno, isn't request: object kinda normal for these middleware-esque patterns?
I.e. if you want to listen to "any message going in/out", there really isn't a common/base interface for all messages.
To your point, unless the user has the type registry option turned on, then they'll at least have the $type key and can do some "if message.$type === ..." checks.
So maybe request: object by default, by request: { $type: string } if the type registry feature is enabled?
Fwiw I'm not super-close to the nuances of object vs. the index signature, so if you've done some WIP coding of these handlers, and the index signature makes the handler code more ergonomic, I'm good with that too!
There was a problem hiding this comment.
ended up going with the following, as after looking up object I realized its still a bit of a wide net:
afterResponse?<T extends { [k in keyof T]: unknown }>(response: T): void;
|
Cool, lgtm, thanks @lukealvoeiro ! |
# [1.164.0](v1.163.0...v1.164.0) (2023-11-09) ### Features * add before and after request methods to base service ([#961](#961)) ([19ba6a5](19ba6a5))
|
🎉 This PR is included in version 1.164.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Consumers of any generated service may want to abstract away functionality like additional error parsing or request/response logging into their Rpc implementation, as this lets them perform this shared logic across all requests and responses they have. This PR implements this change, adding two settings to ts_proto_opt:
false)false)When
true, these will allow you to define an Rpc with two additional, optional methods -beforeRequestandafterResponseTesting
beforeRequestandafterResponseare only called when they are set.Documentation
README.mdto include the two new settings