stephenh
Support for comments in oneof fields
Hello,
I’ve been using ts-proto to generate TypeScript code from my proto files, and while the --comments=true flag works well for normal fields, it does not include JSDoc comments for oneof fields. Specifically, the comments within a oneof block in the proto files are omitted in the generated TypeScript output.
For example, consider the following oneof block in my proto file:
oneof abc_type {
// comment1
aatype atype = 2;
// comment2
bbtype btype = 38;
}
When I generate TypeScript code using ts-proto, the JSDoc comments for aatype and bbtype do not appear in the output. Instead, only the fields themselves are generated like this:
abc_type?:
| { $case: "aatype"; atype: xxx }
| { $case: "bbtype"; btype: xxxx }
| undefined;
However, I would expect the JSDoc comments to be preserved for the fields in the oneof block, similar to how they are for regular fields.
abc_type?:
/**
* comment1.
*/
| { $case: "aatype"; atype: xxx }
/**
* comment2
*/
| { $case: "bbtype"; btype: xxxx }
| undefined;
is there any plan to cover this? Thank you
Hi @sefaphlvn ; it makes sense, including the comments would be a good idea!
Iirc our comment handling code is pretty generic, i.e. once you find the sourceInfo for the given proto AST node, there is a maybeAddComment that will just do the right thing:
maybeAddComment(options, sourceInfo, chunks, messageDesc.options?.deprecated);
// interface name should be defined to avoid import collisions
chunks.push(code`export interface ${def(fullName)} {`);
...ah, actually looks like we've got this comment in the code:
/*
// Ideally we'd put the comments for each oneof field next to the anonymous
// type we've created in the type union above, but ts-poet currently lacks
// that ability. For now just concatenate all comments into one big one.
let comments: Array<string> = [];
const info = sourceInfo.lookup(Fields.message.oneof_decl, oneofIndex);
That was saying "our ts-poet [code generation library] doesn't support comments on anonymous types", which was true in ~2020 when that PR #95 was added, but ts-poet has since had a large refactoring to be "just string literals", so we really should be able to add the comments to the output now.
If you could work on a PR that adds this maybeAddComment, likely by just uncommenting this code, and probably working it into the this unionType map fn:
const unionType = joinCode(
fields.map((f) => {
let fieldName = maybeSnakeToCamel(f.name, options);
let typeName = toTypeName(ctx, messageDesc, f);
let valueName = oneofValueName(fieldName, options);
// do something with maybeAddComment here?...
return code`{ ${mbReadonly}$case: '${fieldName}', ${mbReadonly}${valueName}: ${typeName} }`;
}),
That would be great! Thanks!
I saw the comment of oneOf items in the sourceInfo, but unfortunately I could not find a method to access that comments.
oneof_decl has a value of 8 but the oneOf item comments in sourceInfo start with 2
It would be great if I could get a hint or a method?
Hi @sefaphlvn , sorry for the late reply -- as a very lazy ask, @n3rdyme helped build our initial comment generation back in 2020 :-D , n3rdryme, we couldn't add support for "comments on oneofs" at the time, due to a limitation of ts-poet, but we've since completely redone the ts-poet API, to be just string templates, and so this should be an easy add now.
Only if you're interested/have time, do you mind looking at @sefaphlvn 's question above? He's looking to understand the sourceInfo data structure that we use to generate the comments from.
Thanks!
Hi, @stephenh I have successfully extracted comments for the oneof fields. However, I encountered a rather basic problem: I couldn’t figure out how to force a line break after the | character. Any guidance on achieving this would be greatly appreciated.
function generateOneofProperty(
ctx: Context,
messageDesc: DescriptorProto,
oneofIndex: number,
sourceInfo: SourceInfo,
): Code {
const { options } = ctx;
const fields = messageDesc.field
.map((field, index) => ({ index, field }))
.filter((item) => isWithinOneOf(item.field) && item.field.oneofIndex === oneofIndex);
const mbReadonly = maybeReadonly(options);
const info = sourceInfo.lookup(Fields.message.oneof_decl, oneofIndex);
let outerComments: Code[] = [];
maybeAddComment(options, info, outerComments);
const unionType = joinCode(
fields.flatMap((f) => {
const fieldInfo = sourceInfo.lookup(Fields.message.field, f.index);
let fieldName = maybeSnakeToCamel(f.field.name, options);
let typeName = toTypeName(ctx, messageDesc, f.field);
let valueName = oneofValueName(fieldName, options);
let fieldComments: Code[] = [];
maybeAddComment(options, fieldInfo, fieldComments);
return [
code`|`,
...fieldComments,
code`{ ${mbReadonly}$case: '${fieldName}', ${mbReadonly}${valueName}: ${typeName} }`,
];
}),
{ on: "\n" }
);
const name = maybeSnakeToCamel(messageDesc.oneofDecl[oneofIndex].name, options);
return joinCode(
[
...outerComments,
code`${mbReadonly}${name}?:`,
unionType,
code`| ${nullOrUndefined(options)},`,
],
{ on: "\n" }
);
}
this is output:
/** User information message */
export interface User {
$type: "example.User";
/** Unique user ID */
userId?:
| string
| undefined;
/** Username */
username?:
| string
| undefined;
/** User contact information */
contactInfo?:
| /** User email address */
{ $case: "email"; email: string }
| /**
* User phone number
* asdadas asdasd asdas
*/
{ $case: "phoneNumber"; phoneNumber: string }
| undefined;
/** Field indicating if the user is verified */
isVerified?: boolean | undefined;
}
its should be:
/** User information message */
export interface User {
$type: "example.User";
/** Unique user ID */
userId?:
| string
| undefined;
/** Username */
username?:
| string
| undefined;
/** User contact information */
contactInfo?:
|
/** User email address */
{ $case: "email"; email: string }
|
/**
* User phone number
* asdadas asdasd asdas
*/
{ $case: "phoneNumber"; phoneNumber: string }
| undefined;
/** Field indicating if the user is verified */
isVerified?: boolean | undefined;
}
Hi @sefaphlvn , apologies for the late reply!
The approach that ts-proto's sibling/child project, ts-poet, uses for code generation is to make "a giant unformatted mess of code" and then pass it through dprint, which is a prettier-ish formatter, but written in Rust and so really/much faster (we started with prettier until it because a significant/obvious bottleneck).
dprint has a few more config options that prettier, so there might be one that changes the output:
https://dprint.dev/plugins/typescript/
Here's where we set/pass dprint options to ts-poet:
https://github.com/stephenh/ts-proto/blob/6100390c7bad812730f87a461fa5154e084602cb/src/options.ts#L368
And ts-poet has its own "mimic prettier" set of default dprint options:
https://github.com/stephenh/ts-poet/blob/b2b7ea2c31e9f44a33056baf1cf16b412cd5eb38/src/Code.ts#L253
Fwiw I wouldn't be surprised if there isn't an option to change this behavior in dprint's output, but it's worth looking; if there isn't, is this a deal breaker for you? Or just a nice to have?
Hi, None of the options in dprint were helpful for this issue. I’m reading comments and generating documentation using the TypeScript API, but if a comment appears immediately after the | character in single line, I’m unable to read the comments, likely due to a limitation within the TypeScript API. However, I’ve found a workaround and will proceed with this until a better solution is identified.
const combinedComments = fieldComments.join('\n');
return code`| // \n ${combinedComments} { ${mbReadonly}$case: '${fieldName}', ${mbReadonly}${valueName}: ${typeName} }`;
This produces the following output:
location?:
| //
/** Country */
{ $case: "country"; country: string }
| //
/** State or province */
{ $case: "state"; state: string }
| undefined;
@sefaphlvn that looks great! Let me know when/if you open a PR; unless you already did and I just missed it. Thanks!