tatethurston / TwirpScript

A protobuf RPC framework for JavaScript and TypeScript
MIT License
142 stars 13 forks source link

Compile fails when using message called "Entry" #200

Closed pwalker closed 2 years ago

pwalker commented 2 years ago

I'm encountering a problem when I have a message named "Entry", the compilation fails. Here's an example repo https://github.com/pwalker/twirpscript_repro and I'll paste the proto file in here:

syntax = "proto3";

package test;

service Service {
  rpc Create (CreateRequest) returns (CreateResponse);
}

message CreateRequest {
  string some_id = 1;
  repeated Entry entries = 2;

  message Entry {
    string other_id = 1;
  }
}

message CreateResponse {
  string status = 1;
}

However, if I change Entry to Bar it works:

syntax = "proto3";

package test;

service Service {
  rpc Create (CreateRequest) returns (CreateResponse);
}

message CreateRequest {
  string some_id = 1;
  repeated Bar entries = 2;

  message Bar {
    string other_id = 1;
  }
}

message CreateResponse {
  string status = 1;
}

Also, here's something that could be a related stacktrace? This includes some other fields and things from the actual message I'm trying to use (line 161, 162, 164, 165), but I think the repro case above demonstrates the error.

SyntaxError: Identifier expected. (163:38)
  161 | sourceId:string;
  162 | note:string;
> 163 | entries:Record<string, CreateRequest.['value'] | undefined>;
      |                                      ^
  164 | executionId:string;
  165 | transactionId:string;
  166 | }
    at Ve (/some/path/node_modules/prettier/parser-typescript.js:1:15545)
    at vz (/some/path/node_modules/prettier/parser-typescript.js:280:5919)
    at Object.yz [as parse] (/some/path/node_modules/prettier/parser-typescript.js:280:6242)
    at Object.parse (/some/path/node_modules/prettier/index.js:7334:23)
    at coreFormat (/some/path/node_modules/prettier/index.js:8645:18)
    at formatWithCursor2 (/some/path/node_modules/prettier/index.js:8837:18)
    at /some/path/node_modules/prettier/index.js:37229:12
    at format (/some/path/node_modules/prettier/index.js:37243:12)
    at writeFile (file:///some/path/node_modules/protoscript/codegen/compile.js:18:25)
    at file:///some/path/node_modules/protoscript/codegen/compile.js:27:9

I'm using twirpscript version 0.0.62. Let me know if I can provide any more info, but I suspect this just a naming collision somewhere.

tatethurston commented 2 years ago

Thanks for reporting this @pwalker. This must have presented as a very odd bug 😅. TwirpScript is working around map field detection in google protobuf: https://github.com/protocolbuffers/protobuf/issues/9369. Sorry for the bug, I'll have a fix up shortly.

tatethurston commented 2 years ago

@pwalker This is fixed in v0.0.63: https://github.com/tatethurston/TwirpScript/releases/tag/v0.0.63

pwalker commented 2 years ago

Awesome, this works! Thanks for the bugfix, I appreciate it.