feat: Add generic metadata parameter to the generic service definition interface. #530
Conversation
integration/generic-metadata/hero.ts
Outdated
| }; | ||
|
|
||
| export interface HeroService { | ||
| FindOneHero(request: HeroById, metadata?: GenericMetadata): Promise<Hero>; |
There was a problem hiding this comment.
This is the new field!
integration/generic-metadata/hero.ts
Outdated
| bidirectionalStreamingRequest(service: string, method: string, data: Observable<Uint8Array>): Observable<Uint8Array>; | ||
| } | ||
|
|
||
| export interface Strings { |
There was a problem hiding this comment.
Types used in the new field.
integration/generic-metadata/hero.ts
Outdated
| export interface Strings { | ||
| values: string[]; | ||
| } | ||
| type GenericMetadata = { [key: string]: Strings }; |
There was a problem hiding this comment.
I'm not super-close to how like specific frameworks like GRPC handle metadata, but just naively I was thinking that even this generic type is probably making assumptions about the shape of the metadata, right?
I.e. that it must be a key -> { values: string[] } structure...
Does that come from somewhere official like a protobuf spec?
If it doesn't I was wondering if we could make this even "more generic" by allowing the user to really specify their own type. At first I was thinking a generic, like:
interface HeroService<M> {
findHero(input, metadata: M) {}
}
But then the user would have to pass <M> around; but then I thought we could just take the M type directly, like change:
--ts_proto_opt=addGenericMetadata=true
To be:
--ts_proto_opt=metadataType=Foo@./some-file
Using ts-proto's imp syntax.
And the output would be:
import { Foo } from "./some-file";
interface HeroService {
findHero(input, metadata: Foo) {}
}
Which seems like it'd accomplish truly-custom metadata types (so they could potentially be strongly typed, and so "generic" as in "per company" and not "generic" as in "just strings"), without the overhead of always typing out HeroService<FooMetadata>.
Wdyt?
There was a problem hiding this comment.
The shape of the data is pretty locked into Map<string, string[]> semantics, but we represent as key -> { values: string[] } because you can't build a Map<string, string[]> in protobuf, and we're pretty self-referential about this sort of thing. So perhaps there's just enough variability to justify a custom type :)
I agree with you that the generic approach is a little clunky, and the last approach you settled on is pretty good.
I think we prefer an optional metadata: metadata?: Foo so that server implementers only have to think about metadata if they truly need it. Not sure your thoughts there.
There was a problem hiding this comment.
So perhaps there's just enough variability to justify a custom type
:-)
Ah yeah, I guess if the metadata is coming in over the wire, in an extra/baggage type property, it makes sense it's basically HTTP header style key/value(s). I was thinking maybe a server could inject "other things" like logging or something, but that wouldn't be metadata, so n/m I guess...
the last approach you settled on is pretty good.
Cool!
I think we prefer an optional metadata:
Ah yeah, I agree that's fine with me & what we do now. I was being kind of lazy typing it out, but was also thinking that, well, if the user has gone to the trouble of setting ts-proto_opt=metadataType=Foo@... then "surely" (?) for their specific code output they'd probably be fine / actually prefer a required metadata param, but honestly I'm not sure / was just guessing. Whatever you want to do is great.
fizx
changed the title
# [1.110.0](v1.109.1...v1.110.0) (2022-03-15) ### Features * Add generic metadata parameter to the generic service definition interface. ([#530](#530)) ([0f5525a](0f5525a))
|
🎉 This PR is included in version 1.110.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
If you're using generic service definitions, the lack of a way to pass headers/metadata can be a blocker. Here we add an optional parameter to the interface that is generated.