connectrpc / connect-es

The TypeScript implementation of Connect: Protobuf RPC that works.
https://connectrpc.com/
Apache License 2.0
1.32k stars 75 forks source link

Message serialization and classes vs. plain javascript objects with React Server Components #505

Closed dicarlo2 closed 1 year ago

dicarlo2 commented 1 year ago

Is your feature request related to a problem? Please describe. Today @bufbuild deserialized grpc responses into generated classes based on protobuf. This makes using React Server Components somewhat cumbersome because the objects are not JSON.stringify serializable. Additionally, available serialization methods are relatively untyped. There is no method to produce a PlainMessage<MessageType> which might (?) work with JSON.stringify. Also, toJson produces a JsonValue instead of some type related to the original generated type (e.g. JsonValue<MessageType> similar to PlainMessage<MessageType>).

Here is a concrete example of what we're currently doing today to workaround this:

page.tsx (using Next@13)

export default function Page() {
  const answer = await eliza.say({sentence: "I feel happy."});

  return <AnswerViewWrapper answer={answer.toJson()} />;
}

AnswerViewWrapper.tsx

export interface AnswerViewWrapperProps {
  readonly answer: JsonValue;
}

export function AnswerViewWrapper({ answer }: AnswerViewWrapperProps) {
  return <AnswerView answer={SayResponse.fromJson(answer)} />;
}

AnswerView.tsx

export interface AnswerViewProps {
  readonly answer: SayResponse;
}

export function AnswerView({ answer }: AnswerViewProps) {
  return <span>{answer.sentence}</span>;
}

Describe the solution you'd like Eliminate the need to break type safety, and ideally eliminate the extra serialize/deserialize step. Some options:

  1. Move away from, or have an option for, generating plain javascript objects instead of classes. I'm sure there was a strong rationale for using classes, but generally, I believe that a more functional approach will lead to better compatibility across the stack. Use plain interfaces/data objects and provide functions for manipulating them rather than methods.
  2. Provide better type-safe serialization/deserialization. E.g. toJson(): JsonValue<SayResponse> or toPlainMessage(): PlainMessage<SayResponse>

Describe alternatives you've considered

Additional context Add any other context or screenshots about the feature request here.

smaye81 commented 1 year ago

Hi @dicarlo2. Can you provide a bit more detail on your setup? Are you using Next.js with a custom server? With API Routes? We plan to offer better support for Next.js, so I want to get a better understanding of your environment.

DustinJSilk commented 1 year ago

Im experiencing this issue as well. Im using Qwik framework which can only use serializable values in a response from the server. I can simply return MyMessage.toJson(), however as mentioned this breaks type safety. There isn't any easily accessible interface to cast it as either.

I've created a helper function as a workaround:

export function toJson<T extends Message<T>>(msg: T) {
  return msg.toJson() as PartialMessage<T>
}
travelist commented 1 year ago

I'm using react redux toolkit for managing states and having the same problem described here.

In short, It'd be great if I can use something like AsObject method (+ type definition) that makes the class into a Javascript object with type information.

Context of the problem

The redux state must be serializable, so we cannot always use classes as the state values IIUC.

Defining state types by JsonValue or binary loses the type information, which I want to avoid.

Examples

interface UserState {
  // I want to use the type information of `user_pb.UserProfile` instead of using JsonValue
  profile: PlainMessage<user_pb.UserProfile>
}

const initialState: UserState = {
  // this is not ideal initial state, as `new user_pb.UserProfile()` may not be serializable
  // this makes warning message described in the reference section bellow
  profile: { ...new user_pb.UserProfile() }, 
}

export const getMyProfile = createAsyncThunk('user/fetchProfile', async (thunkAPI) => {
  const response = await userClient.getCurrentUser(new GetCurrentUserRequest())
  return {
    //This causes warning messages described in the reference section bellow
    profile: response!.currentUser 
  }
})

Reference

Can I put functions, promises, or other non-serializable items in my store state? explains why the Redux does not want to have non-serializable items in the state. I can get warning messages that points to this link.

Comments

Btw, this is a great library with a lot of well-defined interfaces 💡 I really thank you all for this project! 🙏

tsingson commented 1 year ago

suggest: use https://github.com/pmndrs/valtio to replace redux.

srikrsna-buf commented 1 year ago

Added toPlainMessage function in @bufbuild/protobuf like the original suggestion.

akosyakov commented 10 months ago

Move away from, or have an option for, generating plain javascript objects instead of classes. I'm sure there was a strong rationale for using classes, but generally, I believe that a more functional approach will lead to better compatibility across the stack. Use plain interfaces/data objects and provide functions for manipulating them rather than methods.

It would be nice if someone elaborate why classes were chosen? Although toPlainMessage does the job, it makes usage of JS SDK in coupling with other frameworks quite a pain.

timostamm commented 10 months ago

It would be nice if someone elaborate why classes were chosen? Although toPlainMessage does the job, it makes usage of JS SDK in coupling with other frameworks quite a pain.

Classes are a good fit for protobuf because schema information is required to serialize/parse, and classes can encapsulate function and schema information (also see the FAQ).

Especially with RSC, I agree that it is cumbersome to convert to a plain message. Did you know that you can easily roll your own client that returns plain messages?

We have an example for a custom client here. This example just modifies the types, but it should be straight-forward to convert to plain message. There is also an example for a client with RxJS observables here.

It would be great to have an example for a plain client as well. Depending on how the ecosystem evolves, we're happy to add clients to @connectrpc/connect. An example would be a great start.

arpitBhalla commented 7 months ago

I'm using it with redux toolkit and redux-saga, tried toPlainMessage, Object.assign, lodash's cloneDeep as well, but it's not working for me. Eveytime it says toJSON failed for ‘value

^ @srikrsna-buf

srikrsna-buf commented 7 months ago

@arpitBhalla Can you share a reproducible example?