-
Notifications
You must be signed in to change notification settings - Fork 1k
Add new RpcRequest, RpcResponse types and their transformer types
#3147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 9ab06b7 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
buffalojoec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
steveluscher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }; | ||
|
|
||
| export type RpcResponseTransformer = { | ||
| <TResponse>(response: RpcResponse<unknown>, request: RpcRequest<unknown>): RpcResponse<TResponse>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should request be optional? I could imagine someone not caring about the input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't need to be used by the response transformer but the caller of a response transformer should always provide it, in case it may be needed. Which is way it's not optional in the signature.
39ddfd0 to
5463398
Compare
e236173 to
1d66210
Compare
5463398 to
7ac67e3
Compare
1d66210 to
1e437d1
Compare
7ac67e3 to
5b947a4
Compare
130c829 to
f240c65
Compare
5b947a4 to
ac5e7f8
Compare
f240c65 to
6c3ae7a
Compare
|
I've added documentation for the new types/functions. |
ac5e7f8 to
8e0c480
Compare
6c3ae7a to
925398e
Compare
Merge activity
|
925398e to
9ab06b7
Compare
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |



Now that we've made room for these new types, we add the following:
RpcRequest: The method name and the params are all we need to create a request. Combining them into a single object makes it easier to play with RPC requests. For instance, we can now have aRpcRequestTransformerinstead of aRpcParamsTransformer.RpcResponse: This is the response abstraction that all the layers of the RPC packages will use moving forward. Instead of a simpleunknownpiece of data, theRpcResponseobject contains two async methods:json: Returns the parsed piece of data.text: Returns the unparsed piece of data (as a string).RpcRequestTransformerandRpcResponseTransformer: Functions that take a request/response and return a new request/response respectively. Note that the response transformer also provides the associated request as a second argument.RpcResponseTransformerFor: Same asRpcResponseTransformerbut we know exactly the type of the return data we expect.createJsonRpcResponseTransformer: This function is a helper that transforms a function of type(json: unknown) => Tto aRpcResponseTransformerFor<T>by wrapping it in ajsonasync function.