protocolbuffers / protobuf-javascript

BSD 3-Clause "New" or "Revised" License
363 stars 67 forks source link

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

Open sayjeyhi opened 3 years ago

sayjeyhi commented 3 years ago

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;
    }
  2. 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
  3. 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]>,
    }
    }
  4. 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 commented 3 years ago

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.

sayjeyhi commented 3 years ago

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.

dibenede commented 2 years ago

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.