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

added module for HTMX WS extension #9

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

h0p3zZ
Copy link
Contributor

@h0p3zZ h0p3zZ commented Sep 14, 2024

No description provided.

Copy link
Owner

@sliemeobn sliemeobn left a comment

Choose a reason for hiding this comment

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

thanks! looking really good.

I'll play around with the demos a bit

Comment on lines +11 to +25
struct WSURLPrefix {
var value: String

public static func ws(_ url: String) -> Self {
.init(value: "ws:\(url)")
}

public static func wss(_ url: String) -> Self {
.init(value: "wss:\(url)")
}
}

static func connect(_ prefixedURL: WSURLPrefix) -> HTMLAttribute {
connect(prefixedURL.value)
}
Copy link
Owner

Choose a reason for hiding this comment

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

this feels a bit too much. isn't just string enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, probably true—couldn't decide on whether to add or not to add it.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it should go - less API surface

Copy link
Owner

@sliemeobn sliemeobn left a comment

Choose a reason for hiding this comment

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

I think the examples should be a bit cleaner - currently it is visually quite messy and not clear what is happening.

If we do a ping-pong, we could add a name text box or something and have the WS route return a greeting or something similar. maybe a random number generator - something that is not so ugly on the UI ; )

Comment on lines +64 to +86
func addWSRoutes(to router: Router<some WebSocketRequestContext>) {
router.ws("echo") { _, _ in
.upgrade([:])
} onUpgrade: { inbound, outbound, _ in
// parse WebSocket frames
for try await frame in inbound {
if frame.opcode == .text, String(buffer: frame.data) == "disconnect", frame.fin == true {
break
}
let opcode: WebSocketOpcode = switch frame.opcode {
case .text: .text
case .binary: .binary
case .continuation: .continuation
}
let frame = WebSocketFrame(
fin: frame.fin,
opcode: opcode,
data: frame.data
)
try await outbound.write(.text("<div id=\"echo\">Received: \(String(buffer: frame.data))</div>"))
}
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

this feels awfully complicated - I am sure there is a cleaner way somehow

@@ -29,4 +29,11 @@ func addRoutes(to app: Application) {
})
)
}

app.webSocket("echo") { req, ws in
print(req)
Copy link
Owner

Choose a reason for hiding this comment

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

good servers don't print ; )

div(.hx.ext(.sse), .sse.connect("/time")) {
p(.sse.swap("time")) { "Server Time:" }
}
// example of using htmx ws
div(.hx.ext(.ws), .ws.connect("/echo"), .hx.vals("{ \"value\": \"TestValue\"}"), .hx.swapOOB(.innerHTML), .hx.target("#echo"), .class("flex justify-between")) {
Copy link
Owner

Choose a reason for hiding this comment

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

for json strings #"..."# would be nicer
or ideally have the data come from a text input or something

Comment on lines 36 to -39

## Future directions

- Add module for [WebSockets extension](https://github.com/bigskysoftware/htmx-extensions/blob/main/src/ws/README.md)
Copy link
Owner

Choose a reason for hiding this comment

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

We should add a little chapter to show there is a WS module now as well

Comment on lines +11 to +25
struct WSURLPrefix {
var value: String

public static func ws(_ url: String) -> Self {
.init(value: "ws:\(url)")
}

public static func wss(_ url: String) -> Self {
.init(value: "wss:\(url)")
}
}

static func connect(_ prefixedURL: WSURLPrefix) -> HTMLAttribute {
connect(prefixedURL.value)
}
Copy link
Owner

Choose a reason for hiding this comment

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

I think it should go - less API surface

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