-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Type error when using socket.timeout.emitWithAck #4925
Comments
You are right, we need to apply the same changes on the client side: #4817 |
Oh sorry, I should post this issue in the client side repo. Thanks for answering, I will close this issue and look forward to the new version client. |
Hello. Do I understand correctly that this is not supported on the client yet? When can I expect a new version? Thanks. |
I can reproduce this error with socket.io version 4.7.5, on server side, with or without timeout() call. await targetUserSocket.emitWithAck("PrivateMessage", content);
But socket.emit() works just fine. |
Describe the bug
When using
socket.timeout.emitWithAck
, even if you setEmitEvents
correctly, the return type will be a wrongany
.To Reproduce
no need for server participation.
Server
Socket.IO client version:
4.7.4
Client
Expected behavior
r2
should also be inferred asPromise<number>
.Platform:
5.3.3
Additional context
The return type of
emitWithAck
is fromPromise<FirstArg<Last<EventParams<EmitEvents, Ev>>>>
, which picks the first argument ofcb
. However,timeout
usesDecorateAcknowledgements
thenPrependTimeoutError
, which prepend aError
type param tocb
, so thisFirstArg
will focus on the error instead of the real return value.Maybe replace
FirstArg
with atype LastArg<T> = T extends (...args: infer Args) => any ? Args extends [] ? never : Args extends [...infer _, infer Last] ? Last : never : never;
will solve this issue? I'm not so familiar with the js logic, so just a hypothesis.By the way, I have found the same problem in #4813 , it can be solved by using socket options
ackTimeout
in the client side (I have already adopted this solution). But I think it's better if this problem can be solved completely.The text was updated successfully, but these errors were encountered: