Support for comments in oneof fields

[sefaphlvn]

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]

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!

[sefaphlvn]

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?

[stephenh]

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!

[sefaphlvn]

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;
}

[stephenh]

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:

ts-proto/src/options.ts

Line 368 in 6100390

dprintOptions: { preferSingleLine: true, lineWidth: 120 },

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?

[sefaphlvn]

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;

[stephenh]

@sefaphlvn that looks great! Let me know when/if you open a PR; unless you already did and I just missed it. Thanks!