stephenh / ts-proto

An idiomatic protobuf generator for TypeScript
Apache License 2.0
2.12k stars 345 forks source link

Support for comments in oneof fields #1122

Open sefaphlvn opened 3 hours ago

sefaphlvn commented 3 hours ago

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

stephenh commented 3 hours ago

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!