Skip to content
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

Update RPC transports to use new types #3148

Merged
merged 1 commit into from
Sep 1, 2024

Conversation

lorisleiva
Copy link
Collaborator

@lorisleiva lorisleiva commented Aug 23, 2024

This PR updates the RPC Transport layer by making sure they return the new RpcResponse interface instead of returning the parsed response data directly.

Copy link

changeset-bot bot commented Aug 23, 2024

🦋 Changeset detected

Latest commit: 700d020

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


type CoalescedRequest = {
readonly abortController: AbortController;
numConsumers: number;
readonly responsePromise: Promise<unknown>;
readonly responsePromise: Promise<RpcResponse | undefined>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be | undefined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's already the current type definition of the coalescer. It just didn't show before because unknown | undefined is equivalent to unknown.

packages/rpc-spec/src/rpc.ts Show resolved Hide resolved
Copy link
Collaborator

@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

@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/add-rpc-request-response-types branch from 1d66210 to 1e437d1 Compare August 26, 2024 19:44
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch from 1e437d1 to 130c829 Compare August 26, 2024 19:48
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch from 130c829 to f240c65 Compare August 26, 2024 20:25
@lorisleiva lorisleiva force-pushed the loris/add-rpc-request-response-types branch from f240c65 to 6c3ae7a Compare August 26, 2024 20:30
@lorisleiva lorisleiva force-pushed the loris/update-rpc-transport branch 2 times, most recently from eeccea5 to 5f13964 Compare August 26, 2024 20:46
@lorisleiva lorisleiva marked this pull request as ready for review August 26, 2024 20:51
Copy link
Collaborator 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:15 PM EDT: Graphite rebased this pull request as part of a merge.
  • Sep 1, 2:16 PM EDT: @lorisleiva merged this pull request with Graphite.

@lorisleiva lorisleiva changed the base branch from loris/add-rpc-request-response-types to graphite-base/3148 September 1, 2024 18:11
@lorisleiva lorisleiva changed the base branch from graphite-base/3148 to master September 1, 2024 18:13
@lorisleiva lorisleiva merged commit e1cb697 into master Sep 1, 2024
8 checks passed
@lorisleiva lorisleiva deleted the loris/update-rpc-transport branch September 1, 2024 18:16
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