Upgrade rpc router internals #178
Conversation
|
Thank you for your review @arnetheduck, I've remove all unsafe serializer. Now the user have to explicitly register their types or custom serializer for json rpc to work. |
json_rpc/private/jrpc_sys.nim
Outdated
| {.gcsafe, raises: [IOError].} = | ||
| w.writeValue JsonRPC2Literal | ||
|
|
||
| proc readValue*(r: var JsonReader[JrpcSys], val: var ResponseError) |
There was a problem hiding this comment.
if we disable automatic object serialization in JrcpSys and then opt in for these few types, I think we can avoid these manual case implementations?
There was a problem hiding this comment.
Right, it's too easy to write custom serializer using the new json-serialization, make me forget to use new features in nim-serialization. Now it's fixed.
| @@ -142,6 +143,11 @@ type | |||
| createJsonFlavor JrpcSys, | |||
| requireAllFields = false | |||
|
|
|||
| ResponseError.useDefaultSerializationIn JrpcSys | |||
There was a problem hiding this comment.
or JrpcSys.useDefaultSerializationFor(ResponseError, RequestTx, ...), right?
My preferred option would actually be type RequestTx {.json: [JrpcSys].} or something along those lines that but I suspect we might run into macro constraints implementing that..
There was a problem hiding this comment.
JrpcSys.useDefaultSerializationFor(ResponseError, RequestTx, ...), right?
No, because only ResponseError using both reader dan writer, while the rest use only reader or writer, not both.
There was a problem hiding this comment.
My preferred option would actually be type RequestTx {.json: [JrpcSys].} or something along those lines that but I suspect we might run into macro constraints implementing that..
I think it's doable. We can use simple template custom pragma instead of macro. The macro part already in nim-serialization. So it's still fall within nim-serialization capabilities.
fix #128
fix #90
fix #151
fix #20
And also completing migration to json-serialization