Add strictly typed events #3822

MaximeKjaer · 2021-03-01T21:46:45Z

The kind of change this PR does introduce

Current behavior

User events

EventEmitter functions, namely on and emit, accept a string as the event name, and any[] as the remaining parameters. This allows users to:

Register listeners that listen to wrong event names
Register listeners that don't match the emitted values

Reserved events

Because the event emitter functions aren't strictly typed, it also means that server.on("connection", (socket: Socket) => { ... }) needs an explicit cast of socket: Socket.

New behavior

User events

TypeScript users can define interfaces for the API between server and client by defining interfaces:

interface Messages {
  hello: string;
}

When creating the Server, users can optionally pass this interface as a type parameter:

const server = new Server<Messages>(httpServer);

With this in place, calls to Socket.emit and Socket.on are strictly typed to only allow emitting and listening to "hello" events:

server.on("connection", (socket) => {
  socket.on("hello", (message) => console.log(message)); // works
  socket.on("goodbye", (message) => console.log(message)); // doesn't type check
});

It's also possible to define different messages for each direction of the connection:

interface ClientToServerMessages {
  ping: void;
}

interface ServerToClientMessages {
  pong: void;
}

const server = new Server<ClientToServerMessages, ServerToClientMessages>(httpServer);
server.on("connection", (socket) => {
  socket.on("ping", () => socket.emit("pong")); // works
  socket.on("pong", () => socket.emit("ping")); // doesn't type check
});

Note that if no type parameter is passed to Server, then events are typed as before.

Reserved events

server.on("connection", socket => { ... }) now correctly infers the Socket type for socket. No cast needed! Other reserved events are now also correctly inferred. Strict typing of reserved events is also present when no type parameters are passed to Server.

This does make this a breaking change if users rely on the on callback arguments being of type any. This was the case in this project's tests, where socket being of typed any was used to access private members of Socket. The fix is to cast as any, which makes the implicit any type explicit.

Other information (e.g. related issues)

Fixes #3742

MaximeKjaer · 2021-03-01T22:53:05Z

lib/index.ts

    return this;
  }

+  public on<


I suppose we would also need to strongly type once, removeListener, etc. Perhaps it might make sense to introduce an abstract class StrictEventEmitter between EventEmitter and Server; this would just override all methods to do all the strict typing, and call super's implementations. What do you think of that approach?

How about adding once() too? I think it should cover most use cases. And we'll add StrictEventEmitter in the future, if some users request it. What do you think?

I ended up moving it all to StrictEventEmitter to avoid duplicating all the overrides of on and once. And I added once, which has the exact same signature.

MaximeKjaer · 2021-03-01T22:55:21Z

test/socket.io.test-d.ts

+
+describe("server", () => {
+  describe("no event map", () => {
+    describe("on", () => {


I wrote this test in the same style as Mocha unit tests (describe / it), but these are not actually run. Perhaps it could make sense to refactor to some other structure, which might be less confusing.

The current format looks fine in my opinion. Maybe with a comment at the top stating that this file is covered by tsd?

Good idea! Done :)

darrachequesne · 2021-03-02T11:39:58Z

@MaximeKjaer I see you've already rebased your PR against the latest changes, nice! 👍

I'll take a look at it as soon as possible, but it looks good at first sight 👌

darrachequesne · 2021-03-03T09:45:10Z

lib/index.ts

    return this;
  }

+  public on<


How about adding once() too? I think it should cover most use cases. And we'll add StrictEventEmitter in the future, if some users request it. What do you think?

darrachequesne · 2021-03-03T09:46:43Z

.github/workflows/ci.yml

      - run: npm test
        env:
          CI: true
+      - run: npm run test:types


What do you think about adding npm run test:types in the npm test command instead? (to prevent surprising CI failures while npm test runs fine locally)

Perfect! I placed it under npm test. I split that command into two subcommands, just to make it a little more readable.

darrachequesne · 2021-03-03T09:48:25Z

test/socket.io.test-d.ts

+
+describe("server", () => {
+  describe("no event map", () => {
+    describe("on", () => {


The current format looks fine in my opinion. Maybe with a comment at the top stating that this file is covered by tsd?

darrachequesne · 2021-03-03T10:01:35Z

lib/index.ts


-export class Server extends EventEmitter {
-  public readonly sockets: Namespace;
+/**


How about moving those declarations in their own file? (completely cosmetic, it's your call 👼 )

I'm all for it! With the addition of StrictEventEmitter I think it made a lot of sense to move it all to a separate file.

removeEventListener does not exist in component-emitter

MaximeKjaer · 2021-03-09T13:29:38Z

test/socket.io.ts

        const clientSocket = client(srv, { reconnection: false });
-        clientSocket.on("connect", function init() {
-          clientSocket.removeListener("connect", init);
+        clientSocket.once("connect", () => {


Improving the typing of client Sockets showed that clientSocket.removeListener does not exist in component-emitter, so this had to be fixed in order to get tests to compile.

Syntax: ```ts interface ClientToServerEvents { "my-event": (a: number, b: string, c: number[]) => void; } interface ServerToClientEvents { hello: (message: string) => void; } const io = new Server<ClientToServerEvents, ServerToClientEvents>(httpServer); io.emit("hello", "world"); io.on("connection", (socket) => { socket.on("my-event", (a, b, c) => { // ... }); socket.emit("hello", "again"); }); ``` The events are not typed by default (inferred as any), so this change is backward compatible. Note: we could also have reused the method here ([1]) to add types to the EventEmitter, instead of creating a StrictEventEmitter class. Related: #3742 [1]: https://github.com/binier/tiny-typed-emitter

darrachequesne · 2021-03-09T23:27:14Z

Merged as 0107510.

I made a few minor updates:

<ListenEvents, EmitEvents> instead of <UserEvents, UserEmitEvents>
BroadcastOperator<EmitEvents> instead of BroadcastOperator<ListenEvents, EmitEvents>
NamespaceReservedEventsMap instead of ServerReservedEventsMap
EventParams<...> = Parameters<Map[Ev]> (so the typings are consistent)

Hope that's OK for you 👼

Anyway, awesome work, thanks a lot ❤️

phiresky · 2021-10-26T10:06:23Z

Hey @MaximeKjaer! I'm the author of typed-socket.io, which I wrote a few years ago and seems pretty similar to this functionality you've now integrated here. I've added a note to the readme that similar functionality is now supported in socket.io itself, but since I'm not a huge socket.io user anymore, I'm not sure what the exact comparison would be. My library is still used by a fair amount of people, so if you have any specific info you think would be useful to my readme, let me know.

Syntax: ```ts interface ClientToServerEvents { "my-event": (a: number, b: string, c: number[]) => void; } interface ServerToClientEvents { hello: (message: string) => void; } const io = new Server<ClientToServerEvents, ServerToClientEvents>(httpServer); io.emit("hello", "world"); io.on("connection", (socket) => { socket.on("my-event", (a, b, c) => { // ... }); socket.emit("hello", "again"); }); ``` The events are not typed by default (inferred as any), so this change is backward compatible. Note: we could also have reused the method here ([1]) to add types to the EventEmitter, instead of creating a StrictEventEmitter class. Related: socketio#3742 [1]: https://github.com/binier/tiny-typed-emitter

MaximeKjaer commented Mar 1, 2021

View reviewed changes

MaximeKjaer force-pushed the typed-events branch from a66b88a to 9f9bdd5 Compare March 2, 2021 10:59

Add typed events

1e2f9cf

MaximeKjaer force-pushed the typed-events branch from 9f9bdd5 to 1e2f9cf Compare March 2, 2021 11:11

Add missing event type arguments

86df1b6

MaximeKjaer force-pushed the typed-events branch from f51c5b8 to 86df1b6 Compare March 2, 2021 14:35

darrachequesne reviewed Mar 3, 2021

View reviewed changes

MaximeKjaer added 7 commits March 5, 2021 13:51

Move npm run test:types command to npm test

7afde1c

Move typed EventEmitter code to new StrictEventEmitter class

3230cf7

Add comment to say test-d file is covered by tsd

055c758

Improve typing of client sockets in tests

36d9613

Fix wrong method call

a7e3035

removeEventListener does not exist in component-emitter

Fix untyped message emit

78502d9

Fix class name in documentation

7e9a378

MaximeKjaer commented Mar 9, 2021

View reviewed changes

MaximeKjaer requested a review from darrachequesne March 9, 2021 13:29

darrachequesne closed this Mar 9, 2021

phiresky mentioned this pull request Oct 26, 2021

Update README.md phiresky/typed-socket.io#3

Merged

ReneWerner87 mentioned this pull request Aug 25, 2023

More strictly typed events #4808

Closed

Add strictly typed events #3822

Add strictly typed events #3822

Uh oh!

Conversation

MaximeKjaer commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The kind of change this PR does introduce

Current behavior

User events

Reserved events

New behavior

User events

Reserved events

Other information (e.g. related issues)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darrachequesne commented Mar 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darrachequesne commented Mar 9, 2021

Uh oh!

phiresky commented Oct 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MaximeKjaer commented Mar 1, 2021 •

edited

Loading