strapi-plugin-io icon indicating copy to clipboard operation
strapi-plugin-io copied to clipboard

fix(io:server): Amend event signature to be identical for every event

Open jack828 opened this issue 2 years ago • 8 comments

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.

jack828 avatar Apr 06 '22 18:04 jack828

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 avatar Apr 07 '22 00:04 ComfortablyCoding

@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.

jack828 avatar Apr 07 '22 09:04 jack828

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 avatar Apr 08 '22 01:04 ComfortablyCoding

@ComfortablyCoding How are we supposed to access the active socket in event handlers without this change?

benjamincanac avatar Apr 15 '22 09:04 benjamincanac

@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.

jack828 avatar Apr 15 '22 09:04 jack828

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.

ComfortablyCoding avatar Apr 15 '22 11:04 ComfortablyCoding

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.

jack828 avatar Apr 15 '22 11:04 jack828

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));

cristiangrojas avatar Oct 08 '22 21:10 cristiangrojas

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.

ComfortablyCoding avatar Aug 24 '23 02:08 ComfortablyCoding

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.

ComfortablyCoding avatar Nov 22 '23 01:11 ComfortablyCoding