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

Server emitWithAck on typed Server has "never" typing #5257

Open
mrjogo opened this issue Dec 10, 2024 · 8 comments
Open

Server emitWithAck on typed Server has "never" typing #5257

mrjogo opened this issue Dec 10, 2024 · 8 comments
Labels
to triage Waiting to be triaged by a member of the team

Comments

@mrjogo
Copy link

mrjogo commented Dec 10, 2024

Describe the bug

emitWithAck doesn't seem to be picking up types correctly, so the type variable Ev is assigned never.

To Reproduce

Socket.IO server version: 4.8.1

Server

import { Server } from "socket.io";

interface ServerToClientEvents {
  someEvent: (callback: (err: Error) => void) => void;
}
interface ClientToServerEvents {}

const io = new Server<ClientToServerEvents, ServerToClientEvents>();
io.on("connection", async (socket) => {
  // Typescript gives error "Argument of type 'string' is not assignable to parameter of type 'never'.ts(2345)"
  await socket.emitWithAck("someEvent");

  // No error
  socket.emit("someEvent", (err) => {});
});

Client

No client code necessary.

Expected behavior
No Typescript error (similar to emit behavior)

Platform:

  • Device: Macbook Air M2
  • OS: MacOS Sonoma 14.6.1

Additional context
Seems to be the same bug as mentioned in the comment #4925 (comment), but different from that issue.

@mrjogo mrjogo added the to triage Waiting to be triaged by a member of the team label Dec 10, 2024
@barroudjo
Copy link

barroudjo commented Dec 14, 2024

I have exactly the same problem

@andjsrk
Copy link

andjsrk commented Jan 22, 2025

I think this is intentional, because:

  1. The parameters of callback have nothing other than Error.
  2. and socket.io explicitly excludes parameters of type Error:
    ? FirstNonErrorArg<Last<Parameters<Map[K]>>> extends void
    ? never
    : K
  3. There is no 'non-Error parameters' so FirstNonErrorArg returns never(or undefined? anyway this does not affect the conclusion) and never extends void is true.
  4. The compiler correctly complains there is no events available.

@mrjogo
Copy link
Author

mrjogo commented Jan 22, 2025

If it's intentional, it's unclear why you wouldn't be able to use a callback signature of (err: Error) => void (or () => void which also gives the same error). It's true that (num: number) => void does not cause the error.

@andjsrk
Copy link

andjsrk commented Jan 22, 2025

Since both (err: Error) => void and () => void are unclear in perspective of "What does the event return?", I think excluding them is reasonable.
If you intended "nothing to return", you could use (err: Error, res: void) => void, which is more explicit.

@mrjogo
Copy link
Author

mrjogo commented Jan 22, 2025

I don't see how they're unclear: callback: (err: Error) => void means "the event handler will call callback(err) with an Error object" (granted, I now realize it's a little bit of a weird case because it should probably be Error | undefined to optionally pass errors back, which the emitWithAck Typescript is fine with), and callback: () => void means "the event handler will call callback() to simply ack that it has received the event".

For the latter, what's more indicative that it's a bug is that emit is perfectly happy with it, while emitWithAck is not. At the least, the behavior should be consistent.

@andjsrk
Copy link

andjsrk commented Jan 22, 2025

emitWithAck (asynchronously) returns value that is specified on the callback, whereas emit does not.

// example
import { Server } from "socket.io"

interface ServerToClientEvents {
  test: (cb: (result: number) => void) => void
}
const io = new Server<{}, ServerToClientEvents>()
io.on("connection", socket => {
  const ackResult = socket.emitWithAck("test")
  // ackResult: Promise<number>
  const emitResult = socket.emit("test", () => {})
  // emitResult: boolean (irrelevant to the definition of events)
})

But, if there is no return type specified, what should the function return? Of course we could return undefined, but it is implicit. If the author intended to require explicit return type, this behavior is reasonable.

@mrjogo
Copy link
Author

mrjogo commented Jan 22, 2025

I understand what you're saying from the "how it's implemented" perspective, but I'm looking at it from the "what would a user expect" perspective. This is a perfectly reasonable thing to want to do and expect it will work:

import { Server } from "socket.io"

interface ServerToClientEvents {
  test: (cb: () => void) => void
}
const io = new Server<{}, ServerToClientEvents>()
io.on("connection", async socket => {
  try {
    await socket.emitWithAck("test")
  catch (socketErr: any) {
    console.error(socketErr)
  }
})

Especially since it's doable in a non-async way:

import { Server } from "socket.io"

interface ServerToClientEvents {
  test: (cb: () => void) => void
}
const io = new Server<{}, ServerToClientEvents>()
io.on("connection", socket => {
  await socket.emit("test", (socketErr) => {
    console.error(socketErr)
  })
})

@andjsrk
Copy link

andjsrk commented Jan 22, 2025

(This comment is not intended to defend the current behavior)
FYI, even you write explicit void, you don't need to write additional code on call site:

interface ServerToClientEvents {
  test: (cb: (result: void) => void) => void
}

// server side
socket.emit("test", () => {})

// client side
socket.on("test", cb => {
  cb() // ok
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to triage Waiting to be triaged by a member of the team
Projects
None yet
Development

No branches or pull requests

3 participants