Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the schema, JSONRPC{Request,Response,Notification,Error} generally extends {Request,Response,Notification,Error}. But then nothing actually consumes the JSONRPCX interface - Request sub-types extend Request; Response sub-types extend Response; etc. In this PR, I fix this confusion by having the sub-types extend the JSONRPC types.
Motivation and Context
JSONRPCX extends X.
SpecificSubtypeX extends X.
Therefore:
JSONRPCX and SpecificSubtypeX are siblings in the type hierarchy. But intuitively, the JSONRPC types are supposed to be the base/foundation. One expects: JSONRPC-SUPER-TYPE -> TYPE -> SUB-TYPE. Maybe there is a reallllly good reason for this.
Concrete example:
(So far, so good....)
OK, let's gooooo:
How Has This Been Tested?
There are no tests of this kind in this repo - I am happy to write tests in whatever style is preferred. The assumption was that
tsc
would catch type errors. But this isn't a type/compilation error - the schema is valid, it just implies that an InitializationRequest is not a JSON-RPC 2.0 Request.If you want to see for yourself, you can paste the existing schema into a IDE/playground (with type-checking), and then observe what happens with these statements:
type _SchemaTest = InitializeRequest extends JSONRPCRequest ? true : never;
Also, Claude was nice enough to provide a little snippet:
interface-compatibility-test.txt
Breaking Changes
As far as I can tell, basically no. It seems to me that each SDK declares the types themselves. I searched on GitHub and couldn't find one instance of anyone importing from this repo directly. So in that sense - no.
Types of changes
Checklist
I will complete the items on the checklist once I get confirmation that this analysis is correct.
Additional context
We probably don't need
Request
if my analysis is correct.Result
is OK because it's a property of a JSONRPCResult. Also I pulled out "Error" into its own interface because it was stylistically aberrant.