-
Notifications
You must be signed in to change notification settings - Fork 12
feat: add generate options #150
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -23,9 +23,53 @@ Or download a prebuilt binary from [releases](./releases). | |||||||||
```bash | ||||||||||
protoc | ||||||||||
--typescript-http_out [OUTPUT DIR] \ | ||||||||||
--typescript-http_opt use_enum_numbers=true,use_multi_line_comment=true | ||||||||||
[.proto files ...] | ||||||||||
``` | ||||||||||
|
||||||||||
#### Support options | ||||||||||
|
||||||||||
```ts | ||||||||||
// UseProtoNames controls the casing of generated field names. | ||||||||||
// If set to true, fields will use proto names (typically snake_case). | ||||||||||
// If omitted or set to false, fields will use JSON names (typically camelCase). | ||||||||||
use_proto_names: bool | ||||||||||
|
||||||||||
// UseEnumNumbers emits enum values as numbers. | ||||||||||
use_enum_numbers: bool | ||||||||||
|
||||||||||
// The method names of service methods naming case. | ||||||||||
// Only work when `UseEnumNumbers=true` | ||||||||||
// opt: | ||||||||||
// camelcase: convert name to lower camel case like `camelCase` | ||||||||||
// pascalcase: convert name to pascalcase like `PascalCase` | ||||||||||
// default is pascalcase | ||||||||||
enum_field_naming: string | ||||||||||
|
||||||||||
// Generate comments as multiline comments. | ||||||||||
// multiline comments: /** ... */ | ||||||||||
// single line comments: // ... | ||||||||||
use_multi_line_comment: bool | ||||||||||
|
||||||||||
// force add `undefined` to message field. | ||||||||||
// default true | ||||||||||
force_message_field_undefinable: bool | ||||||||||
|
||||||||||
// If set to true, body will be JSON.stringify before send | ||||||||||
// default true | ||||||||||
use_body_stringify: bool | ||||||||||
Comment on lines
+58
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I'm all for having this option, I understand it as you are also using that as a way to pass options to the request handler? I would rather see a new argument to the request handler - something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. noop, at this monent, protoc-gen-typescript-http/internal/plugin/servicegen.go Lines 167 to 170 in d11ee37
add this option is remove it, and the default value is if someone like to use |
||||||||||
|
||||||||||
// The method names of service methods naming case. | ||||||||||
// opt: | ||||||||||
// camelcase: convert name to lower camel case like `camelCase` | ||||||||||
// pascalcase: convert name to pascalcase like `PascalCase` | ||||||||||
// default is pascalcase | ||||||||||
service_method_naming: 'camelcase' | 'pascalcase' | ||||||||||
|
||||||||||
// If set to true, field int64 and uint64 will convert to string | ||||||||||
force_long_as_string: bool | ||||||||||
Comment on lines
+69
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this just be called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This name is borrowed from: https://github.com/stephenh/ts-proto I would like to borrow from the existing naming rather than define a new one. Of course, if you really don't like it, I can change it. |
||||||||||
``` | ||||||||||
|
||||||||||
--- | ||||||||||
|
||||||||||
The generated clients can be used with any HTTP client that returns a Promise containing JSON data. | ||||||||||
|
@@ -39,11 +83,17 @@ type Request = { | |||||||||
body: string | null | ||||||||||
} | ||||||||||
|
||||||||||
function fetchRequestHandler({path, method, body}: Request) { | ||||||||||
// This is optional | ||||||||||
type RequestOptions = { | ||||||||||
useCache?: boolean; | ||||||||||
} | ||||||||||
Comment on lines
+86
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where does this come from? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see it now, see comment on the |
||||||||||
|
||||||||||
function fetchRequestHandler({path, method, body}: Request & RequestOptions) { | ||||||||||
return fetch(rootUrl + path, {method, body}).then(response => response.json()) | ||||||||||
} | ||||||||||
|
||||||||||
export function siteClient() { | ||||||||||
return createShipperServiceClient(fetchRequestHandler); | ||||||||||
// This Generics is optional | ||||||||||
return createShipperServiceClient<RequestOptions>(fetchRequestHandler); | ||||||||||
} | ||||||||||
``` |
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.
Why does the comment formatting need to be configurable?
I'd rather pick one of the formats (single/multi) and always use that, not configurable.
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.
because multiline comments is readable in ide tips dialog.

and make comment formatting be configurable is to aviod breaking change(Maybe it's just me worrying too much).