BREAKING: Improve types and type validation #19
Conversation
|
I would vote in favor of merging this. I think this is a huge improvement over the types and utils we had before. |
| isPlainObject, | ||
| JsonSize, | ||
| } from './misc'; | ||
|
|
||
| // Note: This struct references itself, so TypeScript cannot infer the type. |
There was a problem hiding this comment.
This seems like a non-insignificant drawback to using Superstruct. It means that for each kind of thing we want we always have to have a struct and a type to go along with that struct. Ideally, those two things should match (or the type should be a subset of the type that the struct represents). I checked over your types and it does seem like that's the case, however, what if we need to support some other JSON-RPC feature in the future? Someone might not really know how this is set up. Is there any way to synchronize the two things? Like, what happens if the Json type, for instance, is just plain wrong?
There was a problem hiding this comment.
This seems like a non-insignificant drawback to using Superstruct. It means that for each kind of thing we want we always have to have a struct and a type to go along with that struct. [...] what if we need to support some other JSON-RPC feature in the future?
I'm not 100% sure what you mean. The Json struct is a special case here, because it can be recursive. In this case TypeScript is not able to infer the type. That's a limitation in TypeScript, not in Superstruct.
For all of the other structs, the proper type is inferred from the struct.
Is there any way to synchronize the two things? Like, what happens if the Json type, for instance, is just plain wrong?
We could possibly do some kind of testing to make sure that the types match, but I personally don't think it's necessary. These things can be caught during reviews, especially since recursive structs probably aren't so common in JSON-RPC.
There was a problem hiding this comment.
Okay, gotcha. After thinking more about this, I see that the problems I have with this really do have to do more with TypeScript limitations than what Superstruct is exactly doing. That's kind of lame, but nothing much we can do about it now. I guess we'll just have to be careful in reviews as you say.
Following #18, this is an implementation of Superstruct for better validation of JSON-RPC related types. It also improves some JSON-RPC types to better match the specification.
Note that request params are only partially validated, i.e., we check if it's an object or array. Since we can't have types for every JSON-RPC request, this is up to the implementing libraries to validate. This also means that functions like
isValidJsonRpcRequestcannot have a generic parameter, because we cannot guarantee that the generic type matches the actual type of the object.This is a breaking change because it changes the signature of certain types and functions.
Closes #18.