Skip to content
This repository has been archived by the owner on Jul 16, 2024. It is now read-only.

Add WebSocket #323

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

jessekelly881
Copy link
Contributor

No description provided.

Copy link

changeset-bot bot commented Dec 6, 2023

⚠️ No Changeset found

Latest commit: 64f4cf1

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.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

@tim-smart
Copy link
Contributor

Hmm, not sure about the model. I'm also looking at Socket here: https://github.com/Effect-TS/experimental/blob/main/src/Socket.ts

I think we will want a lower level PlatformSocket, and a higher level Socket (which I'm leaning towards Channel for)

@jessekelly881
Copy link
Contributor Author

Ooh. I hadn't seen this. Yeah, wrapping send() and the message stream into one type seems like it could be nice.

@tim-smart
Copy link
Contributor

Updated the model, bit more happy with it now: https://effect-ts.github.io/experimental/modules/Socket.ts.html

@jessekelly881
Copy link
Contributor Author

jessekelly881 commented Dec 7, 2023

I like the fact that it's pull based. Something I hadn't thought of was using a Queue for messages instead of a stream but it makes sense. Why is writer an Effect though(instead of just () => Effect)? Wouldn't it be created with the layer? Or are there circumstances when it makes sense to have more than one?

@tim-smart
Copy link
Contributor

writer is a scoped effect, and will end the writable side of the resource when the scope is closed.

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.

2 participants