tatethurston / TwirpScript

A protobuf RPC framework for JavaScript and TypeScript
MIT License
141 stars 14 forks source link

decodeJSON methods aren't called #161

Closed alexisvisco closed 2 years ago

alexisvisco commented 2 years ago

Hello,

While using a side project I create a message with a field with an underscore workspace_id. For debugging I am using JSON rpc calls and when I retrieve a response it seems like the workspaceId isn't filled, but if I force to workspace_id it works (but there is a compile error).

image

Seems like some methods aren't used.

ListJSON :

export async function ListJSON(
  listRequest: ListRequest,
  config?: ClientConfiguration
): Promise<ListResponse> {
  const response = await JSONrequest<ListResponse>(
    "/run.datahouse.protocol.dashboard.v1.DashboardService/List",
    listRequest,
    config
  );
  return response;
}

List:

export async function List(
  listRequest: ListRequest,
  config?: ClientConfiguration
): Promise<ListResponse> {
  const response = await PBrequest(
    "/run.datahouse.protocol.dashboard.v1.DashboardService/List",
    ListRequest.encode(listRequest),
    config
  );
  return ListResponse.decode(response);
}

Not that in the json implementation there no decode 🤔

Thanks you if you can fix that it will be awesome, again you are doing a nice job !

tatethurston commented 2 years ago

Hey @alexisvisco đź‘‹. Thanks for reporting this -- that was a silly oversight on my end. I'll have a fix published soon.

remicaumette commented 2 years ago

Hello @tatethurston, thanks for your quick answer. I have tried the fix but it does not work.

SyntaxError: Unexpected token o in JSON at position 1
    at JSON.parse (<anonymous>)
    at Object.decodeJSON (workspace.pb.ts:357:12)
    at Object.ListJSON [as list] (workspace.pb.ts:57:23)

Source :

  decodeJSON: function (json: string): ListResponse {
    return ListResponse._readMessageJSON(
      ListResponse.initialize(),
      JSON.parse(json)
    );
  },
tatethurston commented 2 years ago

Thanks @remicaumette, I introduced a regression (double JSON.parsing) in #163. This is fixed by #164.