feat: Allow simultaneous services and generic service definitions #512
Conversation
src/main.ts
Outdated
| : isRepeated(field) | ||
| ? '[]' | ||
| : defaultValue(ctx, field); | ||
| ? '{}' |
There was a problem hiding this comment.
prettier did this, not sure if you want to pick it up cause it does make sense.
There was a problem hiding this comment.
Ah yeah, not sure, but if prettier did it then yeah let's leave it in.
fizx
changed the title
src/main.ts
Outdated
| : isRepeated(field) | ||
| ? '[]' | ||
| : defaultValue(ctx, field); | ||
| ? '{}' |
There was a problem hiding this comment.
Ah yeah, not sure, but if prettier did it then yeah let's leave it in.
src/options.ts
Outdated
| @@ -25,6 +25,7 @@ export enum ServiceOption { | |||
| GRPC = 'grpc-js', | |||
| GENERIC = 'generic-definitions', | |||
| DEFAULT = 'default', | |||
| GENERIC_AND_SERVICES = 'generic-definitions-and-services', | |||
There was a problem hiding this comment.
Hm, kinda think, let's maybe not add a new definition and instead change options.outServices to ServiceOption[]?
I was thinking like outputServices=generic,default, but , is already taken as a separator.
We could list options twice, like outputServices=generic-definitions,outputServices=default... I just made a fix to snakeToCamel that provides a precedent for how to support this "nudged to an array" pattern for options:
Does that seem okay?
|
Still working on the PR right now ... |
|
Question about anticipated behavior when someone does a conflicting option with serviceOption=false,serviceOption=default. I'm leaving it as undefined, but open to alternatives. |
# [1.107.0](v1.106.2...v1.107.0) (2022-03-04) ### Features * Allow simultaneous services and generic service definitions ([#512](#512)) ([680831e](680831e))
|
🎉 This PR is included in version 1.107.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Hi, we have this as a use case at my company because we use the basic service interfaces yet need runtime reflection, and have been running for a while.
This patch is backwards compatible and covered by an integration test.
I'm hoping to upstream. Please let me know how I can make it easier to accept the patch!