socketioxide icon indicating copy to clipboard operation
socketioxide copied to clipboard

Custom parser support

Open Totodore opened this issue 1 year ago • 2 comments

Is your feature request related to a problem? Please describe. It is currently impossible to use external parser like msgpack. Moreover, using msgpack as a parser is a way do solve issue #232 because binary packets with msgpack are sent in one ws frame.

Describe the solution you'd like

  • Add a boxed trait Parser that is held by the SocketIoConfig and set with the with_parser fn
  • The boxed trait parser is then held by the Client to encode/decode packets.

Totodore avatar Jan 17 '24 01:01 Totodore

After considering all the possibilities to solve this issue I came to think that we should simply implement the msgpack parser as an enum based solution.

enum Parser {
    Common(CommongParser),
   #[cfg(feature = "msgpack-parser")]
    MsgPack(MsgPackParser),
}

/// Public API implemented for Parser and delegating the actions either to the 
/// `CommongParser` or the `MsgPackParser` based on a flag in the configuration.

My other points for this solution are:

  • Currently there are only two real parsers implementation (maybe three):
    • https://github.com/socketio/socket.io/discussions/4934.
    • Meaning that exposing a public trait and having separated libs for each parser is not really worth because we can reduce parsers to a fixed set (2 or 3).
  • We won't loose any performance when using the Common parser by default because the enum will be reduced to only one variant (so no branching).
  • Here is a little comment on the two different solutions: https://users.rust-lang.org/t/performance-implications-of-box-trait-vs-enum-delegation/11957/4?u=totodore. It is quite nice to understand the advantages and drawback between these two.

@FabianHummel what are your thoughts on this? As you already started to implement something.

Totodore avatar Jan 29 '24 18:01 Totodore

Using enums sounds good to me, especially because they are on the stack which is far bettern than trait objects. When people want to implement other parsers they could then simply open a PR and try to get them officially added to the parser list.

Honestly, I haven't worked on this PR for a while now, but I'll consider taking a look and try to come up with something that can be worked with. Thanks for the suggestion!

FabianHummel avatar Jan 29 '24 19:01 FabianHummel