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

Introducing channels #3308

Merged
merged 1 commit into from
Oct 4, 2024
Merged

Introducing channels #3308

merged 1 commit into from
Oct 4, 2024

Conversation

steveluscher
Copy link
Collaborator

Summary

Subscriptions are different from one-shot requests in that they need two things: a ‘transport’ to fulfil requests to subscribe to some kind of notifications, but also a ‘channel’ over which to send that subscription and wait for notifications. Channels might be shared by many subscriptions, so they need to be treated as a separate type.

In this PR we create the concept of the RpcSubscriptionsChannel and the RpcSubscriptionsChannelCreator.

Copy link

changeset-bot bot commented Oct 4, 2024

⚠️ No Changeset found

Latest commit: 997f4ca

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Collaborator

@lorisleiva lorisleiva left a comment

Choose a reason for hiding this comment

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

Love the Channel abstraction over both receiving messages and sending them. 👌

Comment on lines +21 to +40
```ts
const rpcSubscriptions =
// Step 1 - Create an `RpcSubscriptions` instance. This may be stateful.
createSolanaRpcSubscriptions(mainnet('wss://api.mainnet-beta.solana.com'));
const response = await rpcSubscriptions
// Step 2 - Call supported methods on it to produce `PendingRpcSubscriptionsRequest` objects.
.slotNotifications({ commitment: 'confirmed' })
// Step 3 - Call the `subscribe()` method on those pending requests to trigger them.
.subscribe({ abortSignal: AbortSignal.timeout(10_000) });
// Step 4 - Iterate over the result.
try {
for await (const slotNotification of slotNotifications) {
console.log('Got a slot notification', slotNotification);
}
} catch (e) {
console.error('The subscription closed unexpectedly', e);
} finally {
console.log('We have stopped listening for notifications');
}
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this example is more suited for rpc-subscriptions than rpc-subscriptions-spec since it's the highest-level API we offer.

Perhaps it would be better to offer examples on how to use the new type here instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I literally just copy-pasted this from @solana/rpc-spec.

I honestly don't understand the differences anymore.

Help me, Obi Wan…

@steveluscher steveluscher force-pushed the 10-02-unhook_all_of_the_subscription_augmenters_so_that_we_can_refactor_and_build_them_back_up_one_by_one branch from ad092cf to e4260af Compare October 4, 2024 17:56
Copy link

socket-security bot commented Oct 4, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] environment +3 4.63 MB react-bot
npm/[email protected] environment +2 339 kB react-bot

🚮 Removed packages: npm/[email protected]

View full report↗︎

Copy link
Collaborator Author

steveluscher commented Oct 4, 2024

Merge activity

  • Oct 4, 11:00 AM PDT: @steveluscher started a stack merge that includes this pull request via Graphite.
  • Oct 4, 11:03 AM PDT: Graphite rebased this pull request as part of a merge.
  • Oct 4, 11:04 AM PDT: @steveluscher merged this pull request with Graphite.

@steveluscher steveluscher changed the base branch from 10-02-unhook_all_of_the_subscription_augmenters_so_that_we_can_refactor_and_build_them_back_up_one_by_one to graphite-base/3308 October 4, 2024 18:01
@steveluscher steveluscher changed the base branch from graphite-base/3308 to master October 4, 2024 18:01
@steveluscher steveluscher merged commit d32e6d6 into master Oct 4, 2024
7 checks passed
@steveluscher steveluscher deleted the 10-02-Introducing_channels branch October 4, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants