Skip to content
This repository has been archived by the owner on Sep 14, 2023. It is now read-only.

Feat/rpc client #339

Merged
merged 3 commits into from
Oct 28, 2022
Merged

Feat/rpc client #339

merged 3 commits into from
Oct 28, 2022

Conversation

kratico
Copy link
Contributor

@kratico kratico commented Oct 28, 2022

Resolves #

  • There is an associated issue (required)
  • The change is described in detail
  • There are new or updated tests validating the change (if applicable)

Description

@kratico kratico changed the base branch from main to rpc-round-2 October 28, 2022 11:16
Comment on lines +72 to +73
const controller = new AbortController();
const { signal } = controller;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with the AbortController we don't need a listener ref to remove the listener

Comment on lines +88 to +98
subscribeWithStop: ClientSubscribeWithStop<SendE, InternalE> = (message, createListenerCb) => {
const stop = () => {
delete this.pendingSubscriptions[message.id];
const activeSubscriptionId = this.activeSubscriptionByMessageId[message.id];
if (activeSubscriptionId) {
delete this.activeSubscriptions[activeSubscriptionId];
}
};
this.pendingSubscriptions[message.id] = createListenerCb(stop);
this.call(message);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll rename this to subscribe and remove the method from line 83

this.activeSubscriptionByMessageId[e.id] = e.result;
}
delete this.pendingSubscriptions[e.id];
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are many ifs here but for subscriptions we need to filter the initialization message so we can setup the listener for the actual subscription messages

Comment on lines +49 to +54
this.pendingSubscriptions[id]?.(e);
delete this.pendingSubscriptions[id];
}
for (const id in this.activeSubscriptions) {
this.activeSubscriptions[id]?.(e);
delete this.activeSubscriptions[id];
Copy link
Contributor Author

@kratico kratico Oct 28, 2022

Choose a reason for hiding this comment

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

as long as the provider communicate any network error through the listeners, this will cover #245 for .call and .subscribe

@kratico kratico marked this pull request as ready for review October 28, 2022 16:53
@kratico kratico requested a review from harrysolovay as a code owner October 28, 2022 16:53
@kratico kratico merged commit 5d36c91 into rpc-round-2 Oct 28, 2022
@kratico kratico deleted the feat/rpc-client branch October 28, 2022 16:54
harrysolovay pushed a commit that referenced this pull request Nov 2, 2022
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.

1 participant