fix: simplify handling useJsonWireFormat=true and fix onlyTypes=true #583
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
oyvindwe
commented
Jun 2, 2022
oyvindwe
commented
- Moved logic for required and implied options into options.ts
- Made onlyTypes=true behave as documented.
oyvindwe
changed the title
stephenh
approved these changes
Jun 2, 2022
| // We should promote onlyTypes to its own documented flag, but just an alias for now | ||
| if (!options.outputJsonMethods && !options.outputEncodeMethods && !options.outputClientImpl && !options.nestJs) { | ||
| // onlyTypes=true implies outputJsonMethods=false,outputEncodeMethods=false,outputClientImpl=false,nestJs=false | ||
| if (options.onlyTypes) { |
There was a problem hiding this comment.
...huh, yeah, not sure why we weren't doing this before. Great fix!
If you want to add test cases to options-test.ts to make sure these don't regress, that'd be great!
There was a problem hiding this comment.
This should probably have been fixed when the option was documented. :)
I'll add test cases!
oyvindwe
commented
Jun 3, 2022
| @@ -396,7 +396,7 @@ Generated code will be placed in the Gradle build directory. | |||
|
|
|||
| - With `--ts_proto_opt=onlyTypes=true`, only types will be emitted, and imports for `long` and `protobufjs/minimal` will be excluded. | |||
|
|
|||
| Note: _This is a combination_ of `outputJsonMethods=false,outputEncodeMethods=false,outputClientImpl=false,nestJs=false` | |||
| This is the same as setting `outputJsonMethods=false,outputEncodeMethods=false,outputClientImpl=false,nestJs=false` | |||
There was a problem hiding this comment.
Not sure if nestJs=false should be included here, as it is false by default, but nestJs=true -> onlyTypes=false.
|
Looks great @oyvindwe , thanks! |
stephenh
pushed a commit
that referenced
this pull request
Jun 3, 2022
## [1.115.2](v1.115.1...v1.115.2) (2022-06-03) ### Bug Fixes * simplify handling useJsonWireFormat=true and fix onlyTypes=true ([#583](#583)) ([6e7f938](6e7f938))
|
🎉 This PR is included in version 1.115.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.