-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Emitting with an array/vector data incorrectly serialized as separate arguments. #225
Comments
That is an interesting find out. |
Unfortunately this is the only way I found to handle multi arguments. We should document that case though. |
Could we just check for vector/tuple payloads and wrap them before sending in the crate? |
|
Perhaps it would make sense to always require a tuple in |
I don't really want to force people to put tuples inside If someone want to work on this don't hesitate :). Things to do are :
|
socketioxide/socketioxide/src/packet.rs Lines 313 to 315 in d890ba4
Value::Array(v) if !v.is_empty() => serde_json::to_string(&vec![
(*e).to_string(),
serde_json::to_string(v).unwrap(),
]), Maybe we can modify it like this. |
There are two issues:
|
Value::Array(v) if !v.is_empty() => {
serde_json::to_string(&json!([(*e).to_string(), v]))
}
Yes, there is still a need for a method that supports multiple parameters. |
There is another way to solve this problem. We can add a few notes to the document. let _ = socket
.within(data.room.clone())
.emit("message", json!([[1, 2, 3, 4]]));
let _ = socket.within(data.room).emit("message", response); |
IMHO, whichever way it goes, there should be separate methods for emitting single or multiple values, and A small issue with adding more methods is that they need to be added on 3 types and also have ack/non-ack variants. It's OK for this but if there are more variations in the future, you'd end up with lots and lots of |
What if someone tries to emit multiple arrays in |
If you wanted to emit two arrays as separate arguments, you'd do The current situation is inconsistent, because if you write let data: serde_json::Value = todo!("get a value from somewhere");
let Some(array) = data.as_array() {
socket.emit([array])?
} else {
socket.emit(data)?
} Tbh I'd still argue for a single // Single argument
socket.emit((1,))?;
// Multiple arguments of same type
socket.emit([1, 2, 3])?;
// Multiple arguments of different types
socket.emit((1, "2", serde_json::json!({ "wrapped": "by tuple" })))?;
// Variable number of arguments
let array = vec![0; random_size];
socket.emit(array);
// Variable number of arguments of different types
let mut array: Vec<serde_json::Value> = vec![];
array.push(1.into());
if some_condition { array.push("2".into()); }
socket.emit(array); With a trait definition like this: trait IntoArguments {
fn into_arguments(self) -> Vec<serde_json::Value>;
}
// Implement this for tuples of many different sizes using a macro
impl <T: Serialize> IntoArguments for (T,) {
// pseudocode implementation, would need error handling
fn into_arguments(self) -> Vec<serde_json::Value> { vec![serde_json::to_value(self)] }
}
impl <T: Serialize> IntoArguments for &[T] {
// pseudocode implementation, would need error handling
fn into_arguments(self) -> Vec<serde_json::Value> { self.iter().map(serde_json::to_value).collect() }
}
impl SocketIo {
fn emit(&self, event: &str, args: impl IntoArguments) -> // etc
} In my opinion writing |
I'm still hesitant on the direction to take to fix this issue.
A solution with an For the moment I'm going to simply document this issue on the |
Describe the bug
As described in title, emitting an event with an array or vector like:
will split elements in the vector and send them as separate arguments.
Clients receive that many arguments instead of the intact array:
To Reproduce
Run a server using socketioxide in rust and start a Javascript or python client.
Emitting an event from server with an array/vector payload.
Expected behavior
The expected resulte would be:
Same behavior for tuple and array.
Versions (please complete the following information):
Additional context
If I wrap the array/vector with an additional array, the results will be fine:
The text was updated successfully, but these errors were encountered: