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

Solid WebSockets API: Missing Unsubscribe! #6

Open
CxRes opened this issue Mar 2, 2020 · 13 comments
Open

Solid WebSockets API: Missing Unsubscribe! #6

CxRes opened this issue Mar 2, 2020 · 13 comments

Comments

@CxRes
Copy link
Member

CxRes commented Mar 2, 2020

How does one unsubscribe to watching a resource without terminating the websocket connection itself?

@MasterJames
Copy link

Try reviewing this maybe?
https://github.com/solid/solid/issues/70

@CxRes
Copy link
Member Author

CxRes commented Mar 11, 2020

@MasterJames I am a bit confused by your comment!

Do you:
a) believe that I can find an answer in solid/solid#70?
or
b) want me to suggest improvements, like others in solid/solid#70?

@MasterJames
Copy link

MasterJames commented Mar 13, 2020

Maybe a little from column A (not knowing the state of affairs) and a little from column B (if it is not working properly in the latest builds, or yes to add comment/feedback to the ideas [? If what sould be obvious is not] to your liking or before it gets more engrained if not fully happening yet).
I guess I dont know much I had assumed if you can subscribe you can just unsubscribe?! My appologies if this is not working nor documented yet, (and you'd get a long ponderous pause and a truly disheartened sigh if things were left hlf bakd like that.)

@CxRes
Copy link
Member Author

CxRes commented Mar 16, 2020

I have looked the spec and the NSS code!

The WebSocket API needs a unsubscribe call!

@Ryuno-Ki
Copy link

I think, a naive API would be to return a function of the subscribe call which allows to unsubscribe.
You can see similiar behaviour with setTimeout or RxJS subscriptions.

However, the removeListener API for Socket.io looks a bit different.

@CxRes
Copy link
Member Author

CxRes commented Mar 16, 2020

IMHO you do not need a complicated API here. Whatever the API, the server will ultimately need to see a message on the websocket (since the socket only carries messages), something of the form unsubscribe <url>. We might as well have the client send that directly on the Websocket.

@csarven
Copy link
Member

csarven commented Apr 22, 2020

Moving this issue to solid/specification for consideration.

@csarven
Copy link
Member

csarven commented Apr 22, 2020

@CxRes

This comment has been minimized.

@csarven

This comment has been minimized.

@csarven csarven transferred this issue from solid/specification Sep 24, 2021
@csarven csarven transferred this issue from solid/notifications-panel Oct 27, 2021
@elf-pavlik
Copy link
Member

elf-pavlik commented Jan 20, 2022

I believe that the current draft defines unidirectional notifications. The resource is specified as topic during subscription and all the further communication is unidirectional.

When it comes to unsubscribing, I see why it is needed for subscription types where the publisher delivers a notification to target specified during the subscription step.

For subscription type where subscriber connects to notifications source, closing connection seems sufficient. Why do you see the need for specific unsubscribe request?

@elf-pavlik
Copy link
Member

@CxRes can we close this issue and pick it up in #145

@CxRes
Copy link
Member Author

CxRes commented Jan 13, 2023

@elf-pavlik The only reason I am reluctant to close this issue is that there are items linked here (see links brought in by @csarven here and in turn in those issue), which form a tangled web. I am happy to continue discussion in #145 and keep the discussion in one place, but I propose closing this along with #145,

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

No branches or pull requests

5 participants