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

Generating map<K, V> on javascript type issue #14

Open
sayjeyhi opened this issue Oct 5, 2021 · 3 comments
Open

Generating map<K, V> on javascript type issue #14

sayjeyhi opened this issue Oct 5, 2021 · 3 comments
Labels
bug Something isn't working javascript P3 triaged Issue has been triaged

Comments

@sayjeyhi
Copy link
Contributor

sayjeyhi commented Oct 5, 2021

What version of protobuf and what language are you using?
Version: master/v3.18
Language: Javascript

What operating system (Linux, Windows, ...) and version? Linux

What runtime / compiler are you using (e.g., python version or gcc version) Nodejs

What did you do?
Steps to reproduce the behavior:

  1. Trying to export type map<K, V> so I created a proto file like this:
message MessageResponse {
  string _id = 1;
  string title = 2;
  bool disabled = 3;
  map<string,string> metadata = 4;
}
  1. Converting it using this some commands like this:
protoc -I=/libraries/protos/ /libraries/protos/src/*.proto /libraries/protos/src/**/*.proto --plugin=/libraries/protos/binaries/linux/protoc-gen-grpc-web --js_out=import_style=commonjs,binary:/libraries/protos/build/typescript --grpc-web_out=import_style=typescript,mode=grpcwebtext:/libraries/protos/build/typescript --experimental_allow_proto3_optional
  1. What I am getting on .ts files is something like:
export namespace MessageResponse {
  export type AsObject = {
    id: string,
    title: string,
    disabled: boolean,
    metadataMap: Array<[string, string]>,
  }
}
  1. Well we can see there is a suffix called Map added to my field name! also the generated type is Array<[string, string]>. but what we were expecting, based on language guide here, where an object of {"k": v, …}, like Record<string, V>

What did you expect to see
An object like: Record<string, string>

What did you see instead?
Array<[string, string]>

@elharo
Copy link

elharo commented Oct 5, 2021

https://developers.google.com/protocol-buffers/docs/reference/javascript-generated#map

Why Record<string, string> instead of Map<string, string>? (perhaps obvious to a JavaScript developer) but yes, this looks contrary to the spec.

@elharo elharo added the bug Something isn't working label Oct 5, 2021
@sayjeyhi
Copy link
Contributor Author

sayjeyhi commented Oct 5, 2021

Based on spec, Map<K, V> sounds better, and that auto-added Map suffix is annoying! I think it is better to let users name their field names. Like what I have here, for metadata, I don't want to have metadataMap; I have types, and everyone using that generated type will understand the return type of this message.

@acozzette acozzette transferred this issue from protocolbuffers/protobuf May 16, 2022
@dibenede dibenede added triaged Issue has been triaged P3 labels Aug 12, 2022
@dibenede
Copy link
Contributor

Agreed that we could generate a better type here. As far as we can tell, grpc-web is just generating the type reflecting what we do.

In the meantime, if you call getMetadataMap() you should get a Map type back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working javascript P3 triaged Issue has been triaged
Projects
None yet
Development

No branches or pull requests

3 participants