Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Add new RpcRequest, RpcResponse types and their transformer types #3147

Merged
merged 1 commit into from
Sep 1, 2024

Conversation

lorisleiva
Copy link
Contributor

@lorisleiva lorisleiva commented Aug 23, 2024

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 a RpcRequestTransformer instead of a RpcParamsTransformer.
  • RpcResponse: This is the response abstraction that all the layers of the RPC packages will use moving forward. Instead of a simple unknown piece of data, the RpcResponse object contains two async methods:
    • json: Returns the parsed piece of data.
    • text: Returns the unparsed piece of data (as a string).
  • RpcRequestTransformer and RpcResponseTransformer: 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 as RpcResponseTransformer but 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) => T to a RpcResponseTransformerFor<T> by wrapping it in a json async function.

Copy link

changeset-bot bot commented Aug 23, 2024

🦋 Changeset detected

Latest 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

Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transformers

Copy link
Contributor

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added via Giphy

packages/rpc-spec/src/__tests__/rpc-shared-test.ts Outdated Show resolved Hide resolved
};

export type RpcResponseTransformer = {
<TResponse>(response: RpcResponse<unknown>, request: RpcRequest<unknown>): RpcResponse<TResponse>;
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@lorisleiva lorisleiva force-pushed the loris/rename-rpc-request branch from 39ddfd0 to 5463398 Compare August 26, 2024 19:24
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch from e236173 to 1d66210 Compare August 26, 2024 19:24
@lorisleiva lorisleiva force-pushed the loris/rename-rpc-request branch from 5463398 to 7ac67e3 Compare August 26, 2024 19:44
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch from 1d66210 to 1e437d1 Compare August 26, 2024 19:44
@lorisleiva lorisleiva force-pushed the loris/rename-rpc-request branch from 7ac67e3 to 5b947a4 Compare August 26, 2024 19:48
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch 2 times, most recently from 130c829 to f240c65 Compare August 26, 2024 20:25
@lorisleiva lorisleiva force-pushed the loris/rename-rpc-request branch from 5b947a4 to ac5e7f8 Compare August 26, 2024 20:30
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch from f240c65 to 6c3ae7a Compare August 26, 2024 20:30
@lorisleiva lorisleiva marked this pull request as ready for review August 26, 2024 20:35
@lorisleiva
Copy link
Contributor Author

I've added documentation for the new types/functions.

Copy link
Contributor Author

lorisleiva commented Sep 1, 2024

Merge activity

  • Sep 1, 2:06 PM EDT: @lorisleiva started a stack merge that includes this pull request via Graphite.
  • Sep 1, 2:12 PM EDT: Graphite rebased this pull request as part of a merge.
  • Sep 1, 2:13 PM EDT: @lorisleiva merged this pull request with Graphite.

@lorisleiva lorisleiva changed the base branch from loris/rename-rpc-request to graphite-base/3147 September 1, 2024 18:08
@lorisleiva lorisleiva changed the base branch from graphite-base/3147 to master September 1, 2024 18:10
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch from 925398e to 9ab06b7 Compare September 1, 2024 18:11
@lorisleiva lorisleiva merged commit 4f87d12 into master Sep 1, 2024
7 checks passed
@lorisleiva lorisleiva deleted the loris/add-rpc-request-response-types branch September 1, 2024 18:13
@github-actions github-actions bot mentioned this pull request Sep 1, 2024
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants