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

Codable messages #62

Open
lmcd opened this issue Feb 27, 2020 · 9 comments
Open

Codable messages #62

lmcd opened this issue Feb 27, 2020 · 9 comments
Labels
enhancement New feature or request
Projects

Comments

@lmcd
Copy link

lmcd commented Feb 27, 2020

Would be great to be able to specify a generic Message type on client.connect, that would auto-encode/decode if Codable conformance is present. This type could default to String.

onText could also be renamed onMessage.

Just a thought.

@MihaelIsaev
Copy link
Member

@lmcd Take a look at https://github.com/MihaelIsaev/AwesomeWS

@lmcd
Copy link
Author

lmcd commented Feb 27, 2020

This looks great, but I think I'd rather have support built right in as is in other parts of Vapor.

@tanner0101 tanner0101 added the enhancement New feature or request label Mar 5, 2020
@tanner0101 tanner0101 added this to To Do in Vapor 4 via automation Mar 5, 2020
@tanner0101
Copy link
Member

I agree, this would be a nice feature to have. Do you have any ideas on what a good API for that might look like @lmcd?

@0xTim
Copy link
Member

0xTim commented Mar 6, 2020

Something like (pseudocode):

extension WebSocket {
    func send(_ data: T, encoder: JSONEncoder = JSONEncoder()) where T: Codable {
        guard let data = try? encoder.encode(data), let dataString = String(data: data, encoding: .utf8) else {
            return
        }
        send(dataString)
    }
}

@ericgroom
Copy link

I'm interested in this bit of functionality, @tanner0101 is this something you are open to accepting contributions on? Do we need to nail down an API first?

@tanner0101
Copy link
Member

Yeah any ideas you have for API would be appreciated.

@ericgroom
Copy link

I've been trying my hand at this, and I have an implementation that works for encoding a message when sending (can make a PR if this in itself is valuable enough), but am finding it much harder to implement some kind of automatic decoding.

I'd imagine the API within Vapor to be something like:

    app.webSocket("socket") { req, ws in
        ws.onMessage(Todo.self) { ws, todo in
            todo.isCompleted.toggle()
            ws.send(todo)
        }
        // fallback
        ws.onText { ws, text in
            let todo = Todo(name: text, completed: false)
            ws.send(todo)
        }
    }

Where you could then register multiple onMessage callbacks for each type. There would be a performance cost iterating through each registered type and attempting to decode the text received, but you may just have to do the same outside of the library.

More importantly, I haven't been able to figure out a way to keep a collection of types and callbacks without losing type information.

@tanner0101
Copy link
Member

We'd probably need to use a serialization format which includes a type identifier, like:

{
    "type": "todo",
    "data": { "completed": false, ... }
}

Then that could automatically lookup the Todo.self onMessage handler before attempting to decode. This all seems pretty opinionated though. We should research how other type-safe languages handle WebSocket messaging.

@MihaelIsaev
Copy link
Member

MihaelIsaev commented Apr 1, 2020

We'd probably need to use a serialization format which includes a type identifier, like:

And now it is the same as Bindable observer in my websocket-kit wrapper 🙂
Works perfectly btw 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Vapor 4
  
Backlog
Development

Successfully merging a pull request may close this issue.

5 participants