Skip to content
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 an event with acknowledgement and timeout breaks the Typescript client-to-server event definitions #1555

Closed
ltomov opened this issue Oct 1, 2022 · 6 comments
Labels
documentation Improvements or additions to documentation

Comments

@ltomov
Copy link

ltomov commented Oct 1, 2022

Describe the bug
Emitting an event using a timeout and acknowledgement causes the acknowledgement function to change its arguments list, so that Error is first argument. This breaks the Typescript definitions. See below for more details:

To Reproduce

Socket.IO client version: 4.5.2

Client

import { io } from "socket.io-client";

interface ClientToServerEvents {
    "withAck": (callback: (data: any) => void) => void;
}
const socket: Socket<ServerToClientEvents, ClientToServerEvents> = io("ws://localhost:3000/", {});

socket.on("connect", () => {
  socket.timeout(5000).emit("withAck", (error, data) => {    // typescript throws here, since the function takes just one parameter as per the ClientToServerEvents interface
        /* If .timeout() is not used, then the first argument is data.
            If .timeout() is used, then first argument is error, second one is data.
            This breaks the ClientToServerEvents interface.
            If you change the ClientToServerEvents interface to have an error argument, 
            then calling the callback from the server breaks, since there data is always first parameter. 
            You need to use different interfaces for client and server.
       */
   });
});

Expected behavior
It should be possible to define one ClientToServerEvents interface and use it both with client and server, irrespective if events are emitted with timeout or not.

@ltomov ltomov added the to triage Waiting to be triaged by a member of the team label Oct 1, 2022
@darrachequesne
Copy link
Member

I could indeed reproduce the issue, thanks for the heads-up.

Any idea on how one could fix this?

@darrachequesne darrachequesne added bug Something isn't working and removed to triage Waiting to be triaged by a member of the team labels Oct 13, 2022
@ltomov
Copy link
Author

ltomov commented Oct 13, 2022

Any idea on how one could fix this?

I actually don't see a way to fix it without some breaking change, such as making the "error" parameter always present, or at least last so it can be made optional.

I'm not sure though, there might be some clever typescript tweak to make it work.

Btw a similar problem exists on the server, I'm not sure if I should raise a new issue or put it here:

interface ServerToClientEvents{
    "withAck": (data: { argName: boolean }, callback: (params: any) => void) => void;
}
const namespacedIO: Namespace<ClientToServerEvents, ServerToClientEvents> = io.of("/some-namespace");
namespacedIO.to("some-room").timeout(5000).emit("withAck", {
     argNameWithTypo: true  // due to the use of .timeout() Typescript no longer checks the arguments here, so you can make a typo in argName and it will still compile. If .timeout() is removed, then the compilation will fail, telling you argNameWithTypo is wrong.
}, (err: any, data: any) => {});

@darrachequesne
Copy link
Member

I think the following should work:

type WithTimeoutAck<isEmitter extends boolean, args extends any[]> = isEmitter extends true ? [Error, ...args] : args;

interface ClientToServerEvents<isEmitter extends boolean = false> {
    withAck: (data: { argName: boolean }, callback: (...args: WithTimeoutAck<isEmitter, [string]>) => void) => void;
}

interface ServerToClientEvents<isEmitter extends boolean = false> {

}

const io = new Server<ClientToServerEvents, ServerToClientEvents<true>>(3000);

io.on("connection", (socket) => {
    socket.on("withAck", (val, cb) => {
        cb("123");
    });
});

const socket: Socket<ServerToClientEvents, ClientToServerEvents<true>> = ioc("http://localhost:3000");

socket.timeout(100).emit("withAck", { argName: true }, (err, val) => {
  // ...
});

Not sure whether we can/should include this in the library though.

darrachequesne added a commit to socketio/socket.io that referenced this issue Oct 13, 2022
Typed events were not applied when calling "io.timeout(...).emit()".

Related: socketio/socket.io-client#1555 (comment)

Reference: https://socket.io/docs/v4/typescript/
darrachequesne added a commit to socketio/socket.io-website that referenced this issue Oct 15, 2022
@darrachequesne
Copy link
Member

Added in the documentation there: https://socket.io/docs/v4/typescript/#emitting-with-a-timeout

@darrachequesne darrachequesne added documentation Improvements or additions to documentation and removed bug Something isn't working labels Oct 15, 2022
@ltomov
Copy link
Author

ltomov commented Oct 15, 2022

That worked, thanks!

However I notice one small thing:

// server-side
socket.to("some-room").timeout(5000).emit("serverEvent", "some-param", (err, dataParam) => {
    // here dataParam is sent as an array, because there are multiple ACKs - one for each member of the room
});

// client-side
socket.timeout(5000).emit("clientEvent", "some-param", (err, dataParam) => {
    // here dataParam is not an array, because there's only one ACK - the server
});

socket.on("serverEvent", (someParam, callback) => {
    callback("data-param"); // if we define a string here, then the dataParam in the server will be a string, instead of the correct string[]
});

So I modified that helper a bit to introduce a new parameter - isServer - must be true in server-side ServerToClient interface and false in all other cases:

type WithTimeoutAck<isServer extends boolean, isSender extends boolean, args extends any[]> = 
    isSender extends true 
        ? isServer extends true 
            ? [Error, [...args]] 
            : [Error, ...args] 
        : isServer extends true 
            ? [args] 
            : args;

@engina
Copy link
Contributor

engina commented Jan 13, 2023

Something like below maybe? @darrachequesne

// actual implementation by systemfault from irc.liberacha #typescript
type PrependTimeoutError<T> = {
  [K in keyof T]: T[K] extends (...args: infer Params) => infer Result
    ? (timeoutErr: Error, ...args: Params) => Result
    : T[K];
};

// https://github.com/socketio/socket.io-client/blob/main/lib/socket.ts#L680
public timeout(timeout: number): Socket<ListenEvents, PrependTimeoutError<EmitEvents>> {
    this.flags.timeout = timeout;
    return this;
  }

darrachequesne pushed a commit that referenced this issue Jan 16, 2023
When emitting with a timeout (added in version 4.4.0), the "err"
argument was not properly typed and would require to split the client
and server typings. It will now be automatically inferred as an Error
object.

Workaround for previous versions:

```ts
type WithTimeoutAck<isEmitter extends boolean, args extends any[]> = isEmitter extends true ? [Error, ...args] : args;

interface ClientToServerEvents<isEmitter extends boolean = false> {
    withAck: (data: { argName: boolean }, callback: (...args: WithTimeoutAck<isEmitter, [string]>) => void) => void;
}

interface ServerToClientEvents<isEmitter extends boolean = false> {

}

const io = new Server<ClientToServerEvents, ServerToClientEvents<true>>(3000);

io.on("connection", (socket) => {
    socket.on("withAck", (val, cb) => {
        cb("123");
    });
});

const socket: Socket<ServerToClientEvents, ClientToServerEvents<true>> = ioc("http://localhost:3000");

socket.timeout(100).emit("withAck", { argName: true }, (err, val) => {
  // ...
});
```

Related: #1555
darrachequesne added a commit to socketio/socket.io that referenced this issue Jan 19, 2023
When emitting with a timeout (added in version 4.4.0), the "err"
argument was not properly typed and would require to split the client
and server typings. It will now be automatically inferred as an Error
object.

Workaround for previous versions:

```ts
type WithTimeoutAck<isEmitter extends boolean, args extends any[]> = isEmitter extends true ? [Error, ...args] : args;

interface ClientToServerEvents<isEmitter extends boolean = false> {
    withAck: (data: { argName: boolean }, callback: (...args: WithTimeoutAck<isEmitter, [string]>) => void) => void;
}

interface ServerToClientEvents<isEmitter extends boolean = false> {

}

const io = new Server<ClientToServerEvents, ServerToClientEvents<true>>(3000);

io.on("connection", (socket) => {
    socket.on("withAck", (val, cb) => {
        cb("123");
    });
});

const socket: Socket<ServerToClientEvents, ClientToServerEvents<true>> = ioc("http://localhost:3000");

socket.timeout(100).emit("withAck", { argName: true }, (err, val) => {
  // ...
});
```

Related: socketio/socket.io-client#1555
dzad pushed a commit to dzad/socket.io that referenced this issue May 29, 2023
Typed events were not applied when calling "io.timeout(...).emit()".

Related: socketio/socket.io-client#1555 (comment)

Reference: https://socket.io/docs/v4/typescript/
dzad pushed a commit to dzad/socket.io that referenced this issue May 29, 2023
When emitting with a timeout (added in version 4.4.0), the "err"
argument was not properly typed and would require to split the client
and server typings. It will now be automatically inferred as an Error
object.

Workaround for previous versions:

```ts
type WithTimeoutAck<isEmitter extends boolean, args extends any[]> = isEmitter extends true ? [Error, ...args] : args;

interface ClientToServerEvents<isEmitter extends boolean = false> {
    withAck: (data: { argName: boolean }, callback: (...args: WithTimeoutAck<isEmitter, [string]>) => void) => void;
}

interface ServerToClientEvents<isEmitter extends boolean = false> {

}

const io = new Server<ClientToServerEvents, ServerToClientEvents<true>>(3000);

io.on("connection", (socket) => {
    socket.on("withAck", (val, cb) => {
        cb("123");
    });
});

const socket: Socket<ServerToClientEvents, ClientToServerEvents<true>> = ioc("http://localhost:3000");

socket.timeout(100).emit("withAck", { argName: true }, (err, val) => {
  // ...
});
```

Related: socketio/socket.io-client#1555
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants