Think of parameter emitted by `update` by RPC server to client
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}
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.