search

ondras / deno-json-rpc

JSON-RPC for Deno
0 stars 0 forks source link

Collaboration #1

Open hediet opened 4 years ago

hediet commented 4 years ago

Hi Ondřej ;) Your library seems to be the first JSON RPC implementation for Deno!

Recently, I developed a fully typed JSON RPC library for TypeScript (NodeJS). You might want to have a look at it (hediet/typed-json-rpc, in future it will just be hediet/json-rpc). You can find an architecture overview at the end of the readme. What do you think?

I still don't know whether Deno is worth the hype, but if it is, it would be great to ensure that its ecosystem is awesome too!

ondras commented 4 years ago

Hi @hediet,

thanks for your comment! I briefly skimmed through your code. I find in interesting and well-written.

I still don't know whether Deno is worth the hype

Me neither. I had the need to implement a small websocket server to synchronize a multiplayer game, so I decided to give Deno a shot. My JSON-RPC implementation closely mirrors this need: to have something compatible up and running, working both in Deno and Browser as well (this turned out to be more complex than I initially thought).

You can find an architecture overview at the end of the readme.

I do not have runtime type checks at all -- I did not have this need, I am not familiar with io-ts and also my TS skills are not that great. Your contracts implementation, for instance, is currently far beyond my mental comprehension :-)

On the other hand, I believe that this contract-based io-ts checking can be actually added as an additional layer to my impl, completely unrelated to the underlying JSON-RPC serialization and semantics.

Recently, I developed a fully typed JSON RPC library for TypeScript (NodeJS).

Yeah, I see that your code is rich. In particular, your https://github.com/hediet/typed-json-rpc/blob/master/typed-json-rpc/src/StreamBasedChannel.ts happens to be very similar to what I have :) However, I focused only on the protocol itself, while you also have extensions that automatically do WebSockets, WebWorkers etc.

I noted that you are missing the batching JSON-RPC feature -- my impl is also not fully doing that. Do you think this feature is worthy of implementation?

t would be great to ensure that its ecosystem is awesome too!

Definitely! :+1:

hediet commented 4 years ago

Thanks for your quick reply ;)

this turned out to be more complex than I initially thought

Why? What has been your experience with Deno? Do you use webpack for the client to bundle your Deno code?

I do not have runtime type checks at all

I think runtime type checks are very important for servers working with untrusted clients. I've seen a lot of server code that iterated over untrusted input using a for loop and the array.length property. You could simply crash the server by sending it { length: 1E99 }. Or even worse, server code that sends the unchecked client input straight to mongo db.

While I really love seperation of concerns, I think in this case, bundling JSON RPC with runtime type checks leads to better user code.

I noted that you are missing the batching JSON-RPC feature -- my impl is also not fully doing that. Do you think this feature is worthy of implementation?

Good catch! I don't know what the performance gains of batching are. I think I'll add rudimentary support so that my library really is json rpc compatible. Besides compatibility reasons, I don't think batching has that much value, but implementing it shouldn't be much work either.

ondras commented 4 years ago

Why? What has been your experience with Deno? Do you use webpack for the client to bundle your Deno code?

My Deno experience is mostly fine. The problem is related to tsc which I use to compile the client-side code -- because tsc does not accept .ts file extensions and Deno requires them. See https://github.com/denoland/deno/issues/3846 for more detail.

I think runtime type checks are very important for servers working with untrusted clients.

Agreed! I currently have the comfort of a server being accessed only by my very own client.

While I really love seperation of concerns, I think in this case, bundling JSON RPC with runtime type checks leads to better user code.

I still believe that it should be possible to operate JSON-RPC without using these and add them incrementally when necessary/feasible. Incremental adoption is one of TypeScript's main goals, after all.

Besides compatibility reasons, I don't think batching has that much value, but implementing it shouldn't be much work either.

Agreed. At least on the server side. The client-side API is going to be slightly more complex in order to make a batched call.