Skip to content

chore(types): make RemoteSocket.data type safe - #4234

Merged
darrachequesne merged 4 commits into
socketio:masterfrom
OrkhanAlikhanov:remote-socket-data-type
Jan 6, 2022
Merged

darrachequesne merged 4 commits into
socketio:masterfrom
OrkhanAlikhanov:remote-socket-data-type

Conversation

@OrkhanAlikhanov

Copy link
Copy Markdown
Contributor

@OrkhanAlikhanov

Copy link
Copy Markdown
Contributor Author

Ah formatting. Will fix soon

@OrkhanAlikhanov
OrkhanAlikhanov force-pushed the remote-socket-data-type branch from 6575691 to b0e7d46 Compare January 3, 2022 21:14
Comment thread lib/index.ts
*
* @public
*/
public fetchSockets(): Promise<RemoteSocket<EmitEvents>[]> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we would have:

const sockets = await io.fetchSockets();

// with the BroadcastOperator
const sockets = await io.in("room-1").fetchSockets<SocketData>();

Is that right?

@OrkhanAlikhanov OrkhanAlikhanov Jan 4, 2022 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. We loss the SocketData type when BroadcastOperator is created, so it has to be specified. Otherwise it will default to any

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, I think it would make more sense to go with your first solution here, so we have the same API with and without operator (like to() or except()).

const sockets = await io.fetchSockets();
const sockets = await io.in("room-1").fetchSockets(); // no additional types

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. Will update the PR

@darrachequesne
darrachequesne merged commit 770ee59 into socketio:master Jan 6, 2022
@darrachequesne

Copy link
Copy Markdown
Member

@OrkhanAlikhanov awesome, thanks a lot 👍

@darrachequesne darrachequesne added this to the 4.4.1 milestone Jan 6, 2022
dzad pushed a commit to dzad/socket.io that referenced this pull request May 29, 2023
@OrkhanAlikhanov
OrkhanAlikhanov deleted the remote-socket-data-type branch September 6, 2023 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants