Skip to content
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

Type name conflict when enum is named Error #64

Open
fryorcraken opened this issue Sep 5, 2022 · 3 comments
Open

Type name conflict when enum is named Error #64

fryorcraken opened this issue Sep 5, 2022 · 3 comments

Comments

@fryorcraken
Copy link

Using the following protobuf definition:

message HistoryResponse {
  enum Error {
    ERROR_NONE_UNSPECIFIED = 0;
    ERROR_INVALID_CURSOR = 1;
  }
  optional Error error = 4;
}

The TypeScript generated has the following typescript error:

src/proto/store_v2beta3.ts:364:21 - error TS2351: This expression is not constructable.
  Type 'typeof Error' has no construct signatures.

364           throw new Error('Protocol error: required field "messages" was not found in object')
                        ~~~~~

src/proto/store_v2beta4.ts:373:21 - error TS2351: This expression is not constructable.
  Type 'typeof Error' has no construct signatures.

373           throw new Error('Protocol error: required field "messages" was not found in object')

This is because typescript thinks that Error refers to the enum and not to the native type Error:

export namespace HistoryResponse {
  export enum Error {
    ERROR_NONE_UNSPECIFIED = 'ERROR_NONE_UNSPECIFIED',
    ERROR_INVALID_CURSOR = 'ERROR_INVALID_CURSOR'
  }

  enum __ErrorValues {
    ERROR_NONE_UNSPECIFIED = 0,
    ERROR_INVALID_CURSOR = 1
  }

  export namespace Error {
    export const codec = () => {
      return enumeration<Error>(__ErrorValues)
    }
  }

  let _codec: Codec<HistoryResponse>

  export const codec = (): Codec<HistoryResponse> => {
    if (_codec == null) {
      _codec = message<HistoryResponse>((obj, writer, opts = {}) => {
        if (opts.lengthDelimited !== false) {
          writer.fork()
        }

        if (obj.messages != null) {
          for (const value of obj.messages) {
            writer.uint32(18)
            WakuMessage.codec().encode(value, writer)
          }
        } else {
          throw new Error('Protocol error: required field "messages" was not found in object')
        }
@achingbrain
Copy link
Member

I'm not sure what a good solution to this is. The generator could have a list of built in type names that it would rename if encountered, but that might be surprising to the user?

Is it a big deal to rename the type in your .proto file since the format on the wire would be the same?

I'm open to suggestions.

@tinytb
Copy link

tinytb commented Oct 18, 2022

It seems reasonable to document the fact that Error is a reserved word, so the TypeScript error is expected behavior. We should also point out the workaround that you can rename the type in your .proto file, without breaking the wire format.

Caveat: I'm new to this project, so this may be completely wrong.

@fryorcraken
Copy link
Author

It seems reasonable to document the fact that Error is a reserved word, so the TypeScript error is expected behavior. We should also point out the workaround that you can rename the type in your .proto file, without breaking the wire format.

Yes, this seems to be the appropriate way. Ensuring it's clearly described in readme so there is no time wasted. I would not go down the route of engineering this one. I open the issue more for doc purposes so that the next user does not end up confused by this error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants