plebbit-js icon indicating copy to clipboard operation
plebbit-js copied to clipboard

Think of parameter emitted by `update` by RPC server to client

Open Rinse12 opened this issue 6 months ago • 1 comments

Full convo here: https://t.me/plebbitjs/35127

I just realized the schema of RPC update emitted to client is pretty bad. It's basically a union of CommentIpfs | CommentUpdate. I think it will cause a problem later on when people start having custom props. We should change it to {comment?: CommentIpfs, commentUpdate?: CommentUpdate}

Rinse12 avatar Aug 22 '25 05:08 Rinse12

The 3 options I think right now are:

{
  "jsonrpc": "2.0",
  "method": "commentUpdateNotification",
  "params": {
    "result": CommentIpfs | CommentUpdate,
    "event": "update",
    "subscription": 1234
  }
}

the first notification would be the CommentIpfs, and subsequent ones would be CommentUpdate. the pros would be that it's an exact mirror of the plebbit-js API, the con would be that it's weird to receive 2 different types from the same event.

{
  "jsonrpc": "2.0",
  "method": "commentUpdateNotification",
  "params": {
    "result": {comment, commentUpdate},
    "event": "update",
    "subscription": 1234
  }
}

the first notification would be the {comment}, and subsequent ones would be {commentUpdate}. the pros would be that it's a bit less weird, but it's still kind of weird, you have to do if (res.comment), do x, else if (res.commentUpdate), do y. I'm not sure what the advantage would be vs solution 1.

another thing that's weird is that our other rpc messages aren't wrapped in an object, so that would be inconsistent.

{
  "jsonrpc": "2.0",
  "method": "commentUpdateNotification",
  "params": {
    "result": Comment,
    "event": "comment",
    "subscription": 1234
  }
}
{
  "jsonrpc": "2.0",
  "method": "commentUpdateNotification",
  "params": {
    "result": CommentUpdate,
    "event": "update",
    "subscription": 1234
  }
}

The first update event would have "event": "comment", and the subsequent would have "event": "update".

the con is that it wouldn't be exactly the same as the plebbit-js, the pros are that it would be easier to parse and handle, no need to do if (res.comment) or if (commentOrCommentUpdate.updatedAt) to know which is which.

I'm undecided between 1 and 3 at the moment. I think 2 seems weird, I don't see having to do if (res.comment) as better than having to do if (commentOrCommentUpdate.updatedAt)

Also with option 1, we probably shouldnt even look at the content of the response, we should rather look if it's the first one or not, since the first one will always be the CommentIpfs, and all the other ones will always be the CommentUpdate. so there can't really be any type confusion.

estebanabaroa avatar Aug 23 '25 01:08 estebanabaroa