strapi-plugin-io
strapi-plugin-io copied to clipboard
fix(io:server): Amend event signature to be identical for every event
Hi,
Thanks for writing this - exactly when I needed it! 😄
I've fixed an inconsistency between event types - IMO, they should be called identically in all cases, to make it easier.
The extra examples should help the new user get going quicker as well.
Hello, I appreciate the PR.
In terms of the ctx data in the emitted server events I agree they should be consistent. I dont see a reason to add the io or socket as a param of the ctx. Is their a use case you have for it? io is accessible via the strapi obj (strapi.$io) and any socket data needed can be emitted from the client.
Examples look great, thanks!
@ComfortablyCoding I do have a use case for socket being present, so that i can access the ID.
io being passed through is just a convenience shorthand for a bit of syntax sugar.
The socket is provided as a param for the connection event, as its the only event where you wont have access to the socket. All other events you should already have access to the socket and can add any of its properties as a param in the emitted data. I dont see the need to add it to the ctx.
I am fine with leaving io there as syntactical sugar.
@ComfortablyCoding How are we supposed to access the active socket in event handlers without this change?
@benjamincanac I have had a draft reply and a to-do task on this pr but due to a family emergency it's on hold for me.
I believe what the repo author means is in an event handler you can get the current socket via io.socket.
On mobile so can't check easily and may be wrong.
Aside from the connection event any other client side event will already have access to the socket and can add it as a data param when emitting.
For example if you want a custom server side join event and need the socket ID, the client emit will look like the the following
socket.emit("join", socket.id, Date.now());
In doing so, the corresponding server side join event will be emitted with the socket ID as the first param.
//...
events:[
name:"join",
handler: ({ strapi }, socketId, joinTime)=> {
strapi.log.info(`socket ${socketId} has joined at ${joinTime}`);
}
]
//...
If I am missing some usecase here where the socket data will not be accessable please let me know.
You are correct. I don't think there's something missing from the existing work that actually necessitates this pr. Off the top of my head (and without current access to any of my uncommitted changes) I think this is just sugar for less frequent and verbose access to io.socket. When I can, I will update the examples in the pr to demonstrate and perhaps remove passing in socket as a parameter to the handler.
If the socket is not being passed down to custom events, how can I do the following:
events: [
{
name: "connection",
handler: ({ strapi }, socket) => {
strapi.log.info(`[io] new connection with id ${socket.id}`)
},
},
{
name: "join-room",
handler: ({ strapi }, socket, roomId) => {
socket.join(roomId)
},
},
]
I've tried by hard-coding the socket in the bootstrap.js file and it works
socket.on(event.name, (...data) => event.handler({ strapi, io: strapi.$io }, socket, ...data));
This should be available in v2, feel free to check it out early using the v2.0.0 branch.
NOTE: As its a major version their are breaking changes.
Closing as this has been resolved with the release of v2.
The event handler now has the socket for all events, see https://strapi-plugin-io.netlify.app/api/plugin-config.html#handler
As this is a major version be sure to check out the updated docs for any additional changes.