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

feat(ws): allow closing active subscriptions #4136

Merged
merged 14 commits into from
May 25, 2023
Merged

feat(ws): allow closing active subscriptions #4136

merged 14 commits into from
May 25, 2023

Conversation

gunhaxxor
Copy link
Contributor

@gunhaxxor gunhaxxor commented Apr 4, 2023

Closes #4133
Closes #4135

🎯 Changes

Don't throw "Operation ended prematurely" when calling complete from outside unsubscribeFunc.
wsClient closes all active subscriptions when wsClient is closed allowing the wsClient to actually close the websocket.

βœ… Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made.

@vercel
Copy link

vercel bot commented Apr 4, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
next-prisma-starter βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback May 6, 2023 0:10am
og-image βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback May 6, 2023 0:10am
www βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback May 6, 2023 0:10am

@vercel
Copy link

vercel bot commented Apr 4, 2023

@Dealerpriest is attempting to deploy a commit to the trpc Team on Vercel.

A member of the Team first needs to authorize it.

@gunhaxxor gunhaxxor changed the title Main Allow server to close subscription. Allow userland to close wsClient while subscriptions are active. Apr 4, 2023
@gunhaxxor gunhaxxor marked this pull request as ready for review April 4, 2023 13:22
KATT
KATT previously approved these changes Apr 6, 2023
Copy link
Member

@KATT KATT left a comment

Choose a reason for hiding this comment

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

LGTM :)

Feel free to request another review once build is passing

@sachinraja
Copy link
Member

looks like this breaks a test, could you take a look @Dealerpriest?

Comment on lines +123 to +129
function closeActiveSubscriptions() {
Object.values(pendingRequests).forEach((req) => {
if (req.type === 'subscription') {
req.callbacks.complete();
}
});
}
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test for this?

Copy link
Member

Choose a reason for hiding this comment

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

I.e. making sure that if you have an active sub & call .close() it should just close

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will give it a shot! What's the purpose/differentiation between the interop tests and the usual ones? At first glance I got the impression they are just a duplication ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason for not merging? I have added/edited tests to account for the new functionality. I think it should be correctπŸ˜ŠπŸ‘

@KATT KATT self-requested a review May 24, 2023 23:52
@KATT KATT changed the title fix(ws): allow closing active subscriptions feat(ws): allow closing active subscriptions May 25, 2023
@KATT KATT enabled auto-merge (squash) May 25, 2023 08:46
@KATT KATT disabled auto-merge May 25, 2023 09:42
@KATT KATT merged commit fd3f409 into trpc:main May 25, 2023
28 checks passed
mastondzn added a commit to mastondzn/synopsisbot that referenced this pull request May 28, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@trpc/client](https://trpc.io)
([source](https://togithub.com/trpc/trpc)) | [`10.27.1` ->
`10.28.0`](https://renovatebot.com/diffs/npm/@trpc%2fclient/10.27.1/10.28.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/compatibility-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/confidence-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
| [@trpc/react-query](https://trpc.io)
([source](https://togithub.com/trpc/trpc)) | [`10.27.1` ->
`10.28.0`](https://renovatebot.com/diffs/npm/@trpc%2freact-query/10.27.1/10.28.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/compatibility-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/confidence-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
| [@trpc/server](https://trpc.io)
([source](https://togithub.com/trpc/trpc)) | [`10.27.1` ->
`10.28.0`](https://renovatebot.com/diffs/npm/@trpc%2fserver/10.27.1/10.28.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/compatibility-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/confidence-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>trpc/trpc</summary>

### [`v10.28.0`](https://togithub.com/trpc/trpc/releases/tag/v10.28.0)

[Compare
Source](https://togithub.com/trpc/trpc/compare/v10.27.3...v10.28.0)

#### What's Changed

- feat(`client`): allow closing active subscriptions by
[@&#8203;Dealerpriest](https://togithub.com/Dealerpriest) in
[trpc/trpc#4136
- feat(`client`): add ability to call `TRPCProxyClient`-methods with
`.apply()` by [@&#8203;atoy40](https://togithub.com/atoy40) in
[trpc/trpc#3973
- feat(`react-query`): allow optional cursor in `useInfiniteQuery` by
[@&#8203;lithdew](https://togithub.com/lithdew) in
[trpc/trpc#4374
- fix(`*`): exclude `*.test.*`-files in build outputs by
[@&#8203;KATT](https://togithub.com/KATT) in
[trpc/trpc#4417

#### New Contributors

- [@&#8203;atoy40](https://togithub.com/atoy40) made their first
contribution in
[trpc/trpc#3973
- [@&#8203;miguelvelasquezdev](https://togithub.com/miguelvelasquezdev)
made their first contribution in
[trpc/trpc#4341
- [@&#8203;lithdew](https://togithub.com/lithdew) made their first
contribution in
[trpc/trpc#4374

**Full Changelog**:
trpc/trpc@v10.27.3...v10.28.0

### [`v10.27.3`](https://togithub.com/trpc/trpc/releases/tag/v10.27.3)

[Compare
Source](https://togithub.com/trpc/trpc/compare/v10.27.2...v10.27.3)

#### What's Changed

- fix(`react-query`): fix `useInfiniteQuery` `placeholderData` types by
[@&#8203;SSHari](https://togithub.com/SSHari) in
[trpc/trpc#4402

#### New Contributors

- [@&#8203;SSHari](https://togithub.com/SSHari) made their first
contribution in
[trpc/trpc#4402

**Full Changelog**:
trpc/trpc@v10.27.2...v10.27.3

### [`v10.27.2`](https://togithub.com/trpc/trpc/releases/tag/v10.27.2)

[Compare
Source](https://togithub.com/trpc/trpc/compare/v10.27.1...v10.27.2)

#### What's Changed

- fix(`next)`: remove conditional hook in `withTRPC()` by
[@&#8203;KATT](https://togithub.com/KATT) in
[trpc/trpc#4410

**Full Changelog**:
trpc/trpc@v10.27.1...v10.27.2

</details>

---

### Configuration

πŸ“… **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

β™» **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

πŸ”• **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/synopsisgg/bot).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMDIuNCIsInVwZGF0ZWRJblZlciI6IjM1LjEwMi40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants